On Wed, Feb 27, 2008 at 10:49:18AM -0800, David Glasser wrote:
> Author: joe
> Date: Wed Feb 27 07:05:53 2008
> New Revision: 29609
...
> > @@ -231,8 +252,13 @@
> > decode_bytes(svn_stringbuf_t *str, const char *data, apr_size_t len,
> > unsigned char *inbuf, int *inbuflen, svn_boolean_t *done)
> > {
> > - const char *p, *find;
> > + const char *p;
> > char group[3];
> > + signed char find;
>
> Why not declare find in the else block below?
You can ask the same question of the original code; I just followed it.
It doesn't make any difference to the object code (or, IMO, the
readability) either way.
> > +
> > + /* Resize the stringbuf to make room for the (approximate) size of
> > + output, to avoid repeated resizes later. */
> > + svn_stringbuf_ensure(str, (len / 4) * 3 + 3);
> >
> > for (p = data; !*done && p < data + len; p++)
> > {
> > @@ -249,9 +275,9 @@
> > }
> > else
> > {
> > - find = strchr(base64tab, *p);
> > - if (find != NULL)
> > - inbuf[(*inbuflen)++] = find - base64tab;
> > + find = reverse_base64[(unsigned char)*p];
>
> Huh, I don't see why this cast is necessary.
If char is signed, *p may be negative, hence reverse_base64[*p] could go
boom.
> > + if (find >= 0)
> > + inbuf[(*inbuflen)++] = find;
>
> Hmm, I would have thought you would have needed a cast from signed
> char to unsigned char here.
Do you know of something compiler-specific which requires that (warnings
maybe)? I don't see why, otherwise; signed->unsigned integer
conversions are well-defined in C.
Regards,
joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-28 14:03:36 CET