[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r29609 - trunk/subversion/libsvn_subr

From: Joe Orton <jorton_at_redhat.com>
Date: Thu, 28 Feb 2008 13:02:51 +0000

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

This is an archived mail posted to the Subversion Dev mailing list.