Login | Register
My pages Projects Community openCollabNet

Discussions > dev [DISABLED] > Re: [PATCH]remove adm_access batons in svn_wc_translated_file3()

subversion
Discussion topic

Back to topic list

Re: [PATCH]remove adm_access batons in svn_wc_translated_file3()

Author julianfoad
Full name Julian Foad
Date 2009-08-12 04:56:41 PDT
Message On Wed, 2009-08-12 at 13:58 +0800, yellow.flying wrote:
> Hey Hyrum, Julian,
>
> I rewrite the patch as you like and have tested it, see below:

Thanks, Huihuang.

I have now reviewed the whole patch and it all looks good to me except
for one thing: the "loggy_path" change.

> Log:
> [[[
> Rip out some adm_access usage in svn_wc_translated_file3().
>
> * subversion/include/svn_wc.h
> (svn_wc_translated_file3): New.
> (svn_wc_translated_file2): Deprecate.
>
> * subversion/libsvn_wc​/deprecated.c
> (svn_wc_translated_file2): Reimplement as a wrapper.
>
> * subversion/libsvn_wc​/translate.c
> (svn_wc__internal_tr​anslated_file): New.
> (svn_wc_translated_file3): New.
> (svn_wc_translated_file2): Remove.
>
> * subversion/libsvn_wc​/translate.h
> (svn_wc__internal_tr​anslated_file): New.
>
> * subversion/libsvn_wc/log.c
> (loggy_path): Add a apr_pool_t * parameter and make it able to deal with absolute paths.
> ]]]


> Index: subversion/libsvn_wc​/deprecated.c
> ====================​====================​====================​=======
[...]
> +svn_error_t *
> +svn_wc_translated_file2(const char **xlated_path,
> + const char *src,
> + const char *versioned_file,
> + svn_wc_adm_access_t *adm_access,
> + apr_uint32_t flags,
> + apr_pool_t *pool)
> +{
> + const char *versioned_abspath;
> + const char *root;
> + const char *tmp_root;
> + svn_wc_context_t *wc_ctx;
> +
> + SVN_ERR(svn_dirent_g​et_absolute(&ver​sioned_abspath, versioned_file, pool));
> + SVN_ERR(svn_wc__cont​ext_create_with_db(​&wc_ctx, NULL,
> + svn_wc__adm_get_db(adm_access),
> + pool));
> +
> + SVN_ERR(svn_wc_trans​lated_file3(xlated_p​ath, src, wc_ctx, versioned_abspath,
> + flags, pool, pool));
> + if (! svn_dirent_is_absolu​te(versioned_file))
> + {
> + SVN_ERR(svn_io_temp_​dir(&tmp_root, pool));
> + if (! svn_dirent_is_child(tmp_root, *xlated_path, pool))
> + {
> + SVN_ERR(svn_dirent_g​et_absolute(&roo​t, "", pool));
> + *xlated_path = svn_dirent_is_child(root, *xlated_path, pool);
> + }
> + }
> +
> + return svn_error_return(svn​_wc_context_destroy(​wc_ctx));
> +}

I wasn't sure at first whether this big "if ()" block really produces
the desired relative path. To check that this wrapper function really
does produce the same result as the previous version, I added some
debugging code to output the result of each call to
svn_wc_translated_file2(), and I compared these results with and without
your patch. They were identical. That's good.

(I compared the results produced during all the tests in diff_tests.py,
log_tests.py, update_tests.py, merge_tests.py.)

> Index: subversion/libsvn_wc/log.c
> ====================​====================​====================​=======
> --- subversion/libsvn_wc/log.c (版本 38693)
> +++ subversion/libsvn_wc/log.c (工作副本)
> @@ -1729,12 +1729,31 @@
> * directory. PATH must not be outside that directory. */
> static const char *
> loggy_path(const char *path,
> - svn_wc_adm_access_t *adm_access)
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool)
> {
> + const char *adm_abspath = NULL;
> + const char *abspath = NULL;
> const char *adm_path = svn_wc_adm_access_pa​th(adm_access);
> - const char *local_path = svn_dirent_is_child(adm_path, path, NULL);
> + const char *local_path = NULL;
> + svn_error_t *err1 = NULL, *err2 = NULL;
>
> - if (! local_path && strcmp(path, adm_path) == 0)
> + err1 = svn_dirent_get_absol​ute(&adm_abspath​, adm_path, pool);
> + if (err1)
> + {
> + svn_error_clear(err1);
> + return NULL;
> + }
> + err2 = svn_dirent_get_absol​ute(&abspath, path, pool);
> + if (err2)
> + {
> + svn_error_clear(err2);
> + return NULL;
> + }
> +
> + local_path = svn_dirent_is_child(​adm_abspath, abspath, NULL);
> +
> + if (! local_path && strcmp(abspath, adm_abspath) == 0)
> local_path = SVN_WC_ENTRY_THIS_DIR;
>
> return local_path;

This function should never return NULL. To catch the possible errors
from "..._get_absolute()" I think you will have to change the function's
prototype to return an error, and provide the result through an output
argument.

Don't initialize variables to NULL when they are going to be initialized
to the correct value later. Doing so prevents compilers and "valgrind"
from detecting use before initialization.

This comment at the top of the file should be updated to mention that
the paths can now be absolute:

/* The svn_wc__loggy_* functions in this section take path arguments
   with the same base as with which the adm_access was opened.
*/

This whole "loggy_path()" change is big enough and independent enough
that it could usefully be a separate patch.


Thanks.
- Julian

« Previous message in topic | 9 of 13 | Next message in topic »

Messages

Show all messages in topic

[PATCH]remove adm_access batons in svn_wc_translated_file3() yellowflying HuiHuang 2009-08-09 23:50:09 PDT
     Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() julianfoad Julian Foad 2009-08-10 06:39:22 PDT
     Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() hwright Hyrum K. Wright 2009-08-10 07:36:53 PDT
     Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() yellowflying HuiHuang 2009-08-10 18:10:11 PDT
         Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() hwright Hyrum K. Wright 2009-08-10 18:18:31 PDT
             Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() julianfoad Julian Foad 2009-08-11 03:31:36 PDT
     Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() yellowflying HuiHuang 2009-08-10 18:30:46 PDT
     Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() yellowflying HuiHuang 2009-08-11 23:00:23 PDT
         Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() julianfoad Julian Foad 2009-08-12 04:56:41 PDT
     Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() yellowflying HuiHuang 2009-08-12 08:18:46 PDT
         Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() julianfoad Julian Foad 2009-08-12 09:41:58 PDT
             Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() julianfoad Julian Foad 2009-08-12 10:37:49 PDT
                 Re: [PATCH]remove adm_access batons in svn_wc_translated_file3() yellowflying HuiHuang 2009-08-13 03:18:19 PDT
Messages per page: