Re: svn commit: r36310 - in trunk/subversion: include libsvn_subr tests/libsvn_subr

On Wed, Mar 4, 2009 at 17:28, Bert Huijben <rhuijben_at_sharpsvn.net> wrote:
>...
> +++ trunk/subversion/libsvn_subr/dirent_uri.c   Wed Mar  4 08:28:30 2009        (r36310)
> @@ -140,6 +140,44 @@ canonicalize_to_upper(char c)
>  }
>  #endif
>
> +/* Calculates the length of the dirent absolute or non absolute root in
> +   DIRENT, return 0 if dirent is not rooted  */
> +static apr_size_t
> +dirent_root_length(const char *dirent, apr_size_t len)
> +{
> +#if defined(WIN32) || defined(__CYGWIN__)
> +  if (len >= 2 && dirent[1] == ':' &&
> +      ((dirent[0] >= 'A' && dirent[0] <= 'Z') ||
> +       (dirent[0] >= 'a' && dirent[0] <= 'z')))

check dirent[1] after the letter check, and you won't have to test the length.

> +    {
> +      return (len > 2 && dirent[2] == '/') ? 3 : 2;

no need to test the length. dirent[2] will be '/' or '\0'.

> +    }
> +
> +  if (len > 2 && dirent[0] == '/' && dirent[1] == '/')

No need to test the length. The checks will stop if dirent[x] == '\0'

>...
> +  if (len >= 1 && dirent[0] == '/')
> +    return 1;
> +
> +  return 0;

dirent[0] will be '/' or '\0' (or somethign else). no need to check
the length. so:

  return dirent[0] == '/';

>...
> @@ -760,6 +824,34 @@ char *svn_dirent_join(const char *base,
>   if (SVN_PATH_IS_EMPTY(component))
>     return apr_pmemdup(pool, base, blen + 1);
>
> +#if defined(WIN32) || defined(__CYGWIN__)
> +  if (component[0] == '/')
> +    {
> +      /* '/' is drive relative on Windows, not absolute like on Posix */
> +      if (dirent_is_rooted(base))
> +        {
> +          /* Join component without '/' to root-of(base) */
> +          blen = dirent_root_length(base, blen);
> +          component++;
> +          clen--;
> +
> +          if (blen == 2 && base[1] == ':') /* "C:" case */
> +            {
> +              char *root = apr_pmemdup(pool, base, 3);
> +              root[2] = '/'; /* We don't need the final '\0' */
> +
> +              base = root;
> +              blen = 3;

I would suggest a local variable at the outer block:

  char newbase[3] = "a:/";

dunno if that compiles. worst case:

  char newbase[] = { 'a', ':', '/' };

and the above code can be:

  if (blen == 2 ...)
    {
      newbase[0] = base[0];
      base = newbase;
      blen = 3;
    }

> +            }
> +
> +          if (clen == 0)
> +            return apr_pstrndup(pool, base, blen);

apr_pstrmemdup(pool, base, blen) is better. it won't look for NUL characters.

>...

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1273174
Received on 2009-03-05 19:30:17 CET