John Peacock wrote:
> Here is part 2; I haven't written a log entry in many months, so please
> be gentle with me... ;-)
I'll sort the log out.
> - svn_subst_keywords_t *keywords;
> + apr_hash_t *keywords = NULL;
Throughout this patch (except for one instance) you initialise the pointer to
NULL, but in all cases it is being passed to svn_wc__get_keywords() which
doesn't require it.
> === subversion/libsvn_wc/translate.c
> ==================================================================
[...]
> -svn_wc__get_keywords (svn_subst_keywords_t **keywords,
> +svn_wc__get_keywords (apr_hash_t **keywords,
This implementation doesn't guarantee to set *KEYWORDS if there are no keywords...
> === subversion/libsvn_wc/translate.h
> ==================================================================
[...]
> + if there is no list, leave KEYWORDS hash empty. ADM_ACCESS
[...]
> +svn_error_t *svn_wc__get_keywords (apr_hash_t **keywords,
... but its doc string says that it does. It's a private function so the
choice is yours to not set it, or set it to NULL, or set it to an empty hash,
but the choice affects subsequent usage. The two types of subsequent usage
involved in this patch are the many calls to svn_subst_copy_and_translate3()
which does take notice of NULL as a special value, and the following single
instance of direct usage:
In subversion/libsvn_wc/translate.c: svn_wc_translated_file():
SVN_ERR (svn_wc__get_keywords (&keywords, vfile, adm_access, NULL, pool));
[...]
if ((style == svn_subst_eol_style_none) && (! keywords) && (! special))
Because of these, I recommend that svn_wc__get_keywords set *KEYWORDS to NULL
if there are none (like it did before).
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Sep 21 02:33:00 2005