You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2006/01/16 15:26:49 UTC

Re: svn commit r18105: factor out 'svn_uuid_generate' function

> Index: /home/julianfoad/src/subversion/subversion/include/svn_types.h
> ===================================================================
> --- /home/julianfoad/src/subversion/subversion/include/svn_types.h      (revision 18104)
> +++ /home/julianfoad/src/subversion/subversion/include/svn_types.h      (revision 18105)
> @@ -605,6 +605,14 @@
>  svn_lock_t *
>  svn_lock_dup (const svn_lock_t *lock, apr_pool_t *pool);
> 
> +/**
> + * Return a formatted universal Universal Unique IDentifier (UUID) string.

Repetition of "universal".

> + *
> + * @since New in 1.4.
> + */
> +const char *
> +svn_uuid_generate (apr_pool_t *pool);

> Index: /home/julianfoad/src/subversion/subversion/libsvn_subr/kitchensink.c
> ===================================================================
[...]
> +const char *
> +svn_uuid_generate (apr_pool_t *pool)
> +{
> +  apr_uuid_t uuid;
> +  char *uuid_str = apr_pcalloc (pool, APR_UUID_FORMATTED_LENGTH + 1);

No need to clear the memory; "palloc" will do nicely.  (I know you just copied 
that from one of the existing uses.)

> +  apr_uuid_get (&uuid);
> +  apr_uuid_format (uuid_str, &uuid);
> +  return uuid_str;
> +}

> Index: /home/julianfoad/src/subversion/subversion/mod_dav_svn/version.c
> ===================================================================
> --- /home/julianfoad/src/subversion/subversion/mod_dav_svn/version.c    (revision 18104)
> +++ /home/julianfoad/src/subversion/subversion/mod_dav_svn/version.c    (revision 18105)
> @@ -251,8 +251,7 @@
>    if (auto_checkout)
>      {
>        dav_resource *res; /* ignored */
> -      apr_uuid_t uuid;
> -      char uuid_buf[APR_UUID_FORMATTED_LENGTH + 1];
> +      const char *uuid_buf;
>        void *data;
>        const char *shared_activity, *shared_txn_name = NULL;
> 
> @@ -298,8 +297,7 @@
>        if (! shared_activity)
>          {
>            /* Build a shared activity for all auto-checked-out resources. */
> -          apr_uuid_get(&uuid);
> -          apr_uuid_format(uuid_buf, &uuid);
> +          uuid_buf = svn_uuid_generate(resource->info->r->pool);
>            shared_activity = apr_pstrdup(resource->info->r->pool, uuid_buf);

This is the only use of the UUID here, so the variable "uuid_buf" and the 
"pstrdup" call are redundant.  Just initialise "shared_activity" directly with 
a new UUID, or at least avoid the "pstrdup".

- Julian


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org