On Fri, 2011-05-27 at 14:28 +0200, Jakub Hrozek wrote:
> On 05/26/2011 06:56 PM, Stephen Gallagher wrote:
>> On Thu, 2011-05-26 at 18:15 +0200, Jakub Hrozek wrote:
>>>
https://fedorahosted.org/sssd/ticket/871
>>>
>>> The second patch adds a couple of unit tests to exercise both the old
>>> behaviour and the multiple spaces.
>>
>>
>> Patch 0001: Nack.
>>
>> "if (w) w = 0;" is redundant. "w = 0;" is sufficient here.
>>
>> Also, while it's unlikely to matter, this code only handles the single
>> whitespace code. It would be wiser if we used isspace() instead.
>>
>> For readability purposes, please convert "i", "e" and
"w" to the "bool"
>> type and use true/false.
>>
>> Finally, please comment this code. It's really difficult to read.
>>
>>
>> Patch 0002: Please add test cases for tabs.
>>
>>
>
> Done except converting "i" to "bool" - that can't be done,
"i" is an
> index, not a flag.
Right, I was overzealous on "i".
Nack. You added a comment /* Add escaped whitespace literally */ and
then immediately ignore what it's saying :)
You assign tmp[i] to be a single-space character instead of the literal
whitespace character received.