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

On Wed, Feb 27, 2008 at 7:05 AM, <joe_at_tigris.org> wrote:
> Author: joe
> Date: Wed Feb 27 07:05:53 2008
> New Revision: 29609
>
> Log:
> * subversion/libsvn_subr/svn_base64.c
> (reverse_base64): Define array.
> (decode_bytes): Optimise firstly by avoiding unnecessary stringbuf
> resizes, and secondly by using a lookup table to decode each
> character, rather than a linear search with strchr.
>
>
> Modified:
> trunk/subversion/libsvn_subr/svn_base64.c
>
> Modified: trunk/subversion/libsvn_subr/svn_base64.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/svn_base64.c?pathrev=29609&r1=29608&r2=29609
> ==============================================================================
> --- trunk/subversion/libsvn_subr/svn_base64.c (original)
> +++ trunk/subversion/libsvn_subr/svn_base64.c Wed Feb 27 07:05:53 2008
> @@ -220,6 +220,27 @@
> out[2] = ((in[2] & 0x3) << 6) | in[3];
> }
>
> +/* Lookup table for base64 characters; reverse_base64[ch] gives a
> + negative value if ch is not a valid base64 character, or otherwise
> + the value of the byte represented; 'A' => 0 etc. */
> +static const signed char reverse_base64[256] = {
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 62, -1, -1, -1, 63,
> +52, 53, 54, 55, 56, 57, 58, 59, 60, 61, -1, -1, -1, -1, -1, -1,
> +-1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14,
> +15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, -1, -1, -1, -1, -1,
> +-1, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40,
> +41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
> +-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1
> +};

Indent the lines of this?

>
> /* Decode a byte string which may or may not be the total amount of
> data being decoded. INBUF and *INBUFLEN carry the leftover bytes
> @@ -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?

> +
> + /* 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 (find >= 0)
> + inbuf[(*inbuflen)++] = find;

Hmm, I would have thought you would have needed a cast from signed
char to unsigned char here.

> if (*inbuflen == 4)
> {
> decode_group(inbuf, group);
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: svn-help_at_subversion.tigris.org
>
>

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org