> On 05/27/2011 02:35 PM, Stephen Gallagher wrote:
> > 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.
>
> New patches attached with a unit test that exercises the above issue.
All commented changes have been done, unit tests work fine.
Ack