You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2004/12/09 18:29:22 UTC

Re: svn commit: r12262 - in branches/locking/subversion: libsvn_fs_fs tests/libsvn_fs

fitz@tigris.org writes:

> Author: fitz
> Date: Thu Dec  9 11:30:23 2004
> New Revision: 12262

> --- branches/locking/subversion/libsvn_fs_fs/lock.c	(original)
> +++ branches/locking/subversion/libsvn_fs_fs/lock.c	Thu Dec  9 11:30:23 2004
> @@ -81,6 +81,16 @@
>    return SVN_NO_ERROR;
>  }
>  
> +static svn_error_t *
> +base_path_to_lock_file (char **base_path,
> +                        svn_fs_t *fs,
> +                        apr_pool_t *pool)
> +{
> +  SVN_ERR (merge_paths (base_path, fs->path, LOCK_ROOT_DIR, pool));
> +  SVN_ERR (merge_paths (base_path, *base_path, LOCK_LOCK_DIR, pool));
> +
> +  return SVN_NO_ERROR;
> +}
>  
>  static svn_error_t *
>  abs_path_to_lock_token_file (char **abs_path,
> @@ -636,6 +646,42 @@
>    return SVN_NO_ERROR;
>  }
>  
> +struct dir_walker_baton
> +{
> +  svn_fs_t *fs;
> +  apr_hash_t *locks;
> +};
> +
> +
> +static svn_error_t *
> +locks_dir_walker (void *baton,
> +                  const char *path,
> +                  const apr_finfo_t *finfo,
> +                  apr_pool_t *pool)
> +{
> +  char *base_path;
> +  const char *rel_path;
> +  svn_lock_t *lock;
> +  struct dir_walker_baton *dir_baton;
> +  dir_baton = (struct dir_walker_baton *)baton;

There is usually no need to cast from void*.

> +  
> +  /* Skip directories. */
> +  if (finfo->filetype == APR_DIR)
> +    return SVN_NO_ERROR;
> +
> +  /* Get the repository-relative path for the lock. */
> +  SVN_ERR (base_path_to_lock_file (&base_path, dir_baton->fs, pool));
> +  rel_path = path + strlen(base_path);

That looks just a little fragile.  It probably works, but base_path is
constructed using apr_filepath_merge while svn_io_dir_walk uses the
svn_path functions, will they always be consistent?

Inconsistent "space before param".

> +
> +  /* Get lock */
> +  SVN_ERR (svn_fs_fs__get_lock_from_path (&lock, dir_baton->fs, 
> +                                          rel_path, pool));
> +
> +  /* Stuff lock in hash, keyed on lock->path */
> +  apr_hash_set (dir_baton->locks, lock->path, APR_HASH_KEY_STRING, lock);
> +
> +  return SVN_NO_ERROR;
> +}
>  
>  
>  svn_error_t *
> @@ -644,6 +690,16 @@
>                        const char *path,
>                        apr_pool_t *pool)
>  {
> -  return svn_error_create (SVN_ERR_UNSUPPORTED_FEATURE, 0,
> -                           "Function not yet implemented.");
> +  char *abs_path;
> +  struct dir_walker_baton baton;
> +  baton.fs = fs;
> +  baton.locks = *locks;
> +
> +  /* Compose the absolute/rel path to PATH */
> +  SVN_ERR (abs_path_to_lock_file(&abs_path, fs, path, pool));

Inconsistent "space before param".

> +
> +  SVN_ERR (svn_io_dir_walk (abs_path, APR_FINFO_TYPE, locks_dir_walker,
> +                            (void *)&baton, pool));

There is usually no need to cast to void*.

> +
> +  return SVN_NO_ERROR;
>  }

-- 
Philip Martin

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

Re: svn commit: r12262 - in branches/locking/subversion: libsvn_fs_fs tests/libsvn_fs

Posted by Philip Martin <ph...@codematters.co.uk>.
"Brian W. Fitzpatrick" <fi...@collab.net> writes:

> On Dec 9, 2004, at 12:29 PM, Philip Martin wrote:
>
>> fitz@tigris.org writes:
>>> +
>>> +  /* Skip directories. */
>>> +  if (finfo->filetype == APR_DIR)
>>> +    return SVN_NO_ERROR;
>>> +
>>> +  /* Get the repository-relative path for the lock. */
>>> +  SVN_ERR (base_path_to_lock_file (&base_path, dir_baton->fs, pool));
>>> +  rel_path = path + strlen(base_path);
>>
>> That looks just a little fragile.  It probably works, but base_path is
>> constructed using apr_filepath_merge while svn_io_dir_walk uses the
>> svn_path functions, will they always be consistent?
>
> Hmmm.  Any suggestions?  I initially used svn_path_join_many, but
> shied away since the docstring said to only use it for "internal"
> canonicalized paths.

My first concern was how do I know that rel_path points into path and
not into arbitrary memory beyond the null.  It was while attempting to
determine the algorithm that relates path and base_path that I
realised they use different libraries.  I suppose you could use
strncmp to verify that base_path matches the start of path.

-- 
Philip Martin

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

Re: svn commit: r12262 - in branches/locking/subversion: libsvn_fs_fs tests/libsvn_fs

Posted by "Brian W. Fitzpatrick" <fi...@collab.net>.
On Dec 9, 2004, at 12:29 PM, Philip Martin wrote:

> fitz@tigris.org writes:
>> +
>> +  /* Skip directories. */
>> +  if (finfo->filetype == APR_DIR)
>> +    return SVN_NO_ERROR;
>> +
>> +  /* Get the repository-relative path for the lock. */
>> +  SVN_ERR (base_path_to_lock_file (&base_path, dir_baton->fs, pool));
>> +  rel_path = path + strlen(base_path);
>
> That looks just a little fragile.  It probably works, but base_path is
> constructed using apr_filepath_merge while svn_io_dir_walk uses the
> svn_path functions, will they always be consistent?

Hmmm.  Any suggestions?  I initially used svn_path_join_many, but shied 
away since the docstring said to only use it for "internal" 
canonicalized paths.

All other fixes applied in r12265.

Thanks!

-Fitz


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