Re: [PATCH] Fix svn_log_changed_path_dup()

Madan U Sreenivasan wrote:

> === subversion/libsvn_subr/constructors.c
> ==================================================================
> --- subversion/libsvn_subr/constructors.c (revision 600)
> +++ subversion/libsvn_subr/constructors.c (local)
> @@ -60,12 +60,16 @@
> svn_log_changed_path_t *new_changed_path
> = apr_palloc(pool, sizeof(*new_changed_path));
>
> + /* Copy action */
> *new_changed_path = *changed_path;
>
> - if (new_changed_path->copyfrom_path)
> - new_changed_path->copyfrom_path =
> - apr_pstrdup(pool, new_changed_path->copyfrom_path);
> + /* Copy the copyfrom_path member variable */
> + new_changed_path->copyfrom_path =
> + apr_pstrdup(pool, changed_path->copyfrom_path);

apr_pstrdup() will SEGFAULT if you try to copy a NULL value.
changed_path->copyfrom_path must be NULL if new_changed_path->copyfrom_path
was NULL due to the structure copy that precedes it (see below about that).

>
> + /* Copy the copyfrom_rev member variable */
> + new_changed_path->copyfrom_rev = changed_path->copyfrom_rev;
> +

The copyfrom_rev member has already been copied by virtual of this line up
above:

    *new_changed_path = *changed_path;

This statement in C effectively copies, though not deeply, all the members
of change_page structure into the new_changed_path structure. Any of those
members that aren't pointers are therefore fully copied. What follows after
that statement is performing the deep copies necessary for only the
pointer-y things.

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand