Re: svn commit: rev 300 - trunk/subversion/include trunk/subversion/libsvn_subr trunk/subversion/tests/libsvn_subr

On Wed, Oct 24, 2001 at 11:19:24AM -0500, cmpilato@tigris.org wrote:
>...
> +svn_stringbuf_t *svn_path_is_child (const svn_stringbuf_t *path1,
> + const svn_stringbuf_t *path2,

With new functions, can we get them to use "const svn_string_t *" ??

Continuing to use stringbuf for stuff like this is just going to increase
the pain later.

>...
> +static svn_boolean_t
> +char_is_uri_safe (const unsigned char c)

No need for unsigned, is there? And const is kinda weird for a pass-by-value
argument :-) (certainly doesn't hurt tho)

> +{
> + unsigned char other[18] = "/:.-_!~'()@=+$,&*"; /* sorted by estimated use? */

If you do:

    static const other[] = "...";

Then you won't have to copy the contents on function entry(!!), and the size
will be automatically computed.

>...
> + /* Is this a supported non-alphanumeric character? */
> + for (i = 0; i < sizeof (other); i++)

That sizeof() is going to include the null-term because of the above
definitions.

However, it would probably be faster to do:

    if (strchr("...", c) != NULL)
        return TRUE;

>...
> + /* Now, sprintf() in our escaped character, making sure our
> + buffer is big enough to hold the '%' and two digits. */
> + svn_stringbuf_ensure (retstr, retstr->len + 3);
> + sprintf (retstr->data + retstr->len, "%%%02X", c);

Note that sprintf() can be dog-slow on some boxes. For simple formatting, it
can sometimes be handy to manually insert characters. I doubt we need perf
here, so no biggy. Just bringing up the point.

>...
> +svn_stringbuf_t *
> +svn_path_uri_decode (const svn_stringbuf_t *path, apr_pool_t *pool)
> +{
> + svn_stringbuf_t *retstr;
> + int i;
> + unsigned char c;

Why that whole unsigned again? Note that str->data is (possibly) signed.

>...
> + {
> + unsigned char digitz[3];
> + digitz[0] = path->data[++i];
> + digitz[1] = path->data[++i];
> + digitz[2] = '\0';
> + c = (unsigned char)(strtol (digitz, NULL, 16));

And that unsigned stuff again :-)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org