Re: svn commit: r1042294 - /subversion/trunk/subversion/libsvn_subr/checksum.c

[Stefan Sperling]
> > - is_zeros |= (*checksum)->digest[i];
> > + is_nonzero |= ((char *)(*checksum)->digest)[i] = x1 << 4 | x2;
>
> At the very least, this needs parenthesis unless you want everyone
> to pull out their copy of K&R to check operator precedence rules.

Can do. x1 << 4 | x2 didn't seem too ambiguous to me, but I can see
how it might be.

> Can we do one assignment per line instead? Maybe that's easier to parse.

Hmmm.

      ((char *)(*checksum)->digest)[i] = x1 << 4 | x2;
      is_nonzero |= ((char *)(*checksum)->digest)[i];

...The repeated expression is complex, so you have to visually verify
that it is indeed identical. That seems harder, to me. Or did you
mean:

      is_nonzero |=
        ((char *)(*checksum)->digest)[i] = x1 << 4 | x2;

...Which doesn't seem much easier to parse either.

Peter
Received on 2010-12-05 12:15:06 CET