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

Re: svn commit: r36434 - in trunk/subversion: libsvn_wc tests/libsvn_wc

From: Greg Stein <gstein_at_gmail.com>
Date: Mon, 9 Mar 2009 21:29:43 +0100

On Mon, Mar 9, 2009 at 21:13, Hyrum K. Wright <hyrum_at_hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_wc/entries.c        Mon Mar  9 13:13:01 2009        (r36434)
>...
> @@ -1248,6 +1259,14 @@ read_entries(svn_wc_adm_access_t *adm_ac
>       if (checksum)
>         entry->checksum = svn_checksum_to_cstring(checksum, result_pool);
>
> +     if (lock)
> +       {
> +         entry->lock_token = apr_pstrdup(result_pool, lock->token);
> +         entry->lock_owner = apr_pstrdup(result_pool, lock->owner);
> +         entry->lock_comment = apr_pstrdup(result_pool, lock->comment);
> +         entry->lock_creation_date = lock->date;

No need for the strdups. All this stuff was allocated in result_pool
by the read_info() call.

>...
> +++ trunk/subversion/libsvn_wc/wc_db.c  Mon Mar  9 13:13:01 2009        (r36434)
>...
> @@ -2095,6 +2107,9 @@ svn_wc__db_read_info(svn_wc__db_status_t
>   SVN_ERR(svn_sqlite__bindf(stmt_act, "is", pdh->wc_id, local_relpath));
>   SVN_ERR(svn_sqlite__step(&have_act, stmt_act));
>
> +  if (lock)
> +    *lock = NULL;

No need for this. You do it further below.
(and this doesn't follow precedent)

>...
> @@ -2209,6 +2224,25 @@ svn_wc__db_read_info(svn_wc__db_status_t
>             *repos_relpath = svn_sqlite__column_text(stmt_base, 3,
>                                                      result_pool);
>         }
> +      if (lock)
> +        {
> +          if (!svn_sqlite__column_is_null(stmt_base, 14))
> +            {

No need to change, but will mention: most of the code gets the "super
short" branch out of the way first, then moves into the long stuff.
ie. test for null, set NULL, then the else block handles the longer
part.

>...
> +++ trunk/subversion/libsvn_wc/wc_db.h  Mon Mar  9 13:13:01 2009        (r36434)
> @@ -170,6 +170,22 @@ typedef enum {
>
>  } svn_wc__db_status_t;
>
> +/** Lock information.  We write/read it all as one, so let's use a struct
> +    for convenience.  */
> +typedef struct {
> +  /* The lock token */
> +  const char *token;
> +
> +  /* The owner of the lock */
> +  const char *owner;
> +
> +  /* A comment about the lock, possibly NULL */
> +  const char *comment;

The owner can be NULL, too, from all the code you've written.

>...
> @@ -1002,6 +1019,8 @@ svn_wc__db_read_info(svn_wc__db_status_t
>                      svn_boolean_t *base_shadowed,  /* ### WORKING shadows a
>                                                        ### deleted BASE? */
>
> +                     svn_wc__db_lock_t **lock,
> +
>                      svn_wc__db_t *db,
>                      const char *local_abspath,
>                      apr_pool_t *result_pool,

svn_wc__db_base_get_info() should be able to fetch locks, too.

>...

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1298735
Received on 2009-03-09 21:30:00 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.