Re: svn commit: rev 461 - trunk/subversion/libsvn_wc

Looks good overall, although a lot of extra work is being done rather than
using features of the existing functions.

Also: it appears that the wcprops did not get the extension added to them. I
haven't looked into the code, but I still have ".svn/wcprops/foo.c" in my
working copy. So they still appear in a "find" :-)

And I apologize for not reviewing the patch beforehand. I would have seen
the comments below:

On Thu, Nov 15, 2001 at 12:26:46PM -0600, kevin@tigris.org wrote:
>...
> --- OLD/trunk/subversion/libsvn_wc/props.c Thu Nov 15 12:26:44 2001
> +++ NEW/trunk/subversion/libsvn_wc/props.c Thu Nov 15 12:26:44 2001
>...
> @@ -625,12 +625,14 @@
> SVN_WC__ADM_PROP_BASE,
> name->data,
> NULL);
> + svn_stringbuf_appendcstr(tmp_prop_base, SVN_WC__BASE_EXT);
> real_prop_base = svn_wc__adm_path (svn_stringbuf_create ("", pool),
> 0, /* no tmp */
> pool,
> SVN_WC__ADM_PROP_BASE,
> name->data,
> NULL);
> + svn_stringbuf_appendcstr(real_prop_base, SVN_WC__BASE_EXT);

The above two chunks of code would be simplified by simply placing
SVN_WC__BASE_EXT directly into the svn_wc__adm_path() call. No need for a
second function call to append the extension.

>...
> --- OLD/trunk/subversion/libsvn_wc/adm_files.c Thu Nov 15 12:26:45 2001
> +++ NEW/trunk/subversion/libsvn_wc/adm_files.c Thu Nov 15 12:26:46 2001
> @@ -339,6 +339,7 @@
> {
> svn_stringbuf_t *newpath, *basename;
> svn_path_split (path, &newpath, &basename, svn_path_local_style, pool);
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> return sync_adm_file (newpath,
> pool,
> SVN_WC__ADM_TEXT_BASE,

Same here.

>...
> +svn_stringbuf_t *
> +svn_wc__text_base_path (const svn_stringbuf_t *path,
> + svn_boolean_t tmp,
> + apr_pool_t *pool)
> {
> svn_stringbuf_t *newpath, *basename;
> svn_path_split (path, &newpath, &basename, svn_path_local_style, pool);
> -
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> extend_with_adm_name (newpath,
> 0,
> pool,
> tmp ? SVN_WC__ADM_TMP : "",
> - thing,
> + SVN_WC__ADM_TEXT_BASE,
> basename->data,
> NULL);

And here.

>...
> @@ -714,6 +707,7 @@
> {
> svn_stringbuf_t *newpath, *basename;
> svn_path_split (path, &newpath, &basename, svn_path_local_style, pool);
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> return open_adm_file (handle, newpath, flags, pool,
> SVN_WC__ADM_TEXT_BASE, basename->data, NULL);

And here.

Notice a pattern? :-)

(sorry that I didn't get the patch reviewed beforehand :-( )

> }
> @@ -727,6 +721,7 @@
> {
> svn_stringbuf_t *newpath, *basename;
> svn_path_split (path, &newpath, &basename, svn_path_local_style, pool);
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> return close_adm_file (fp, newpath, write, pool,
> SVN_WC__ADM_TEXT_BASE, basename->data, NULL);

And here.

> }
> @@ -790,8 +785,11 @@
> return open_adm_file (handle, parent_dir, flags, pool,
> SVN_WC__ADM_DIR_PROP_BASE, NULL);
> else
> - return open_adm_file (handle, parent_dir, flags, pool,
> - SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + {
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> + return open_adm_file (handle, parent_dir, flags, pool,
> + SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + }

Again.

> }
> else if (wcprops)
> {
> @@ -850,8 +848,11 @@
> return close_adm_file (fp, parent_dir, sync, pool,
> SVN_WC__ADM_DIR_PROP_BASE, NULL);
> else
> - return close_adm_file (fp, parent_dir, sync, pool,
> - SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + {
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> + return close_adm_file (fp, parent_dir, sync, pool,
> + SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + }

Another

> }
> else if (wcprops)
> {
> @@ -909,8 +910,11 @@
> return sync_adm_file (parent_dir, pool,
> SVN_WC__ADM_DIR_PROP_BASE, NULL);
> else
> - return sync_adm_file (parent_dir, pool,
> - SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + {
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> + return sync_adm_file (parent_dir, pool,
> + SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + }

and yet another...

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org