On Thu, Sep 17, 2009 at 01:49:22PM -0700, Dave Brown wrote:
> Stefan Sperling wrote:
> >
> > This does not conform to our coding style.
> > Should be:
> >
> > if (! (*logy_path))
> > {
> > ...
> > }
>
> Properly styled patch attached. Thanks for pointing it out, Stefan.
> Index: subversion/libsvn_wc/log.c
> ===================================================================
> --- subversion/libsvn_wc/log.c (revision 39412)
> +++ subversion/libsvn_wc/log.c (working copy)
> @@ -1709,8 +1709,15 @@
> SVN_ERR(svn_dirent_get_absolute(&abspath, path, pool));
> *logy_path = svn_dirent_is_child(adm_abspath, abspath, NULL);
>
> - if (! (*logy_path) && strcmp(abspath, adm_abspath) == 0)
> - *logy_path = SVN_WC_ENTRY_THIS_DIR;
> + if (! (*logy_path) )
We usually don't put spaces between parentheses like this.
> + {
> + if (strcmp(abspath, adm_abspath) == 0) /* same path */
> + *logy_path = SVN_WC_ENTRY_THIS_DIR;
> + else /* not a child path */
> + return svn_error_createf(SVN_ERR_BAD_RELATIVE_PATH, NULL,
> + "path '%s' not a child of '%s'\n",
> + path, adm_abspath);
When using paths in user-visible output, you need to wrap them
in svn_dirent_local_style() so that e.g. the path separators
come out right depending on the platform.
I've fixed those two issues locally, and the resulting diff looks like:
Index: subversion/libsvn_wc/log.c
===================================================================
--- subversion/libsvn_wc/log.c (revision 39413)
+++ subversion/libsvn_wc/log.c (working copy)
@@ -1709,8 +1709,16 @@ loggy_path(const char **logy_path,
SVN_ERR(svn_dirent_get_absolute(&abspath, path, pool));
*logy_path = svn_dirent_is_child(adm_abspath, abspath, NULL);
- if (! (*logy_path) && strcmp(abspath, adm_abspath) == 0)
- *logy_path = SVN_WC_ENTRY_THIS_DIR;
+ if (! (*logy_path))
+ {
+ if (strcmp(abspath, adm_abspath) == 0) /* same path */
+ *logy_path = SVN_WC_ENTRY_THIS_DIR;
+ else /* not a child path */
+ return svn_error_createf(SVN_ERR_BAD_RELATIVE_PATH, NULL,
+ "path '%s' not a child of '%s'\n",
+ svn_dirent_local_style(path, pool),
+ svn_dirent_local_style(adm_abspath, pool));
+ }
return SVN_NO_ERROR;
}
This looks good to me.
I will run make check on this and commit it if it does not cause
any tests to fail.
Thanks!
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2396229
Received on 2009-09-18 01:01:08 CEST