Re: [PATCH 04/45] pathspec: add copy_pathspec

2013-06-12 Thread Junio C Hamano
Eric Sunshine  writes:

> On Sun, Jun 9, 2013 at 2:25 AM, Nguyễn Thái Ngọc Duy  
> wrote:
>> The function is made to use with free_pathspec() because a simple
>> struct assignment is not enough (free_pathspec wants to free "items"
>> pointer).
>
> I had to read this about five or six times before I could parse and
> understand it. Perhaps rewrite something like this:
>
> free_pathspec() frees pathspec->items, hence simple struct
> assignment does not properly duplicate a pathspec. Introduce
> copy_pathspec() to clone a pathspec correctly by deeply copying
> the items member.

Thanks.  It might be worth that this is not a full "deep" copy,
perhaps like this:

pathspec: add copy_pathspec

Because free_pathspec wants to free "items" pointer in the pathspec
structure, a simple structure assignment is not enough if you want
to copy an existing pathspec into another.  Freeing the original will
damage the copy unless a deep copy is made.

Note that the strings in pathspec->items->match and the array
pathspec->raw[] are still shared between the original and the
copy.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 04/45] pathspec: add copy_pathspec

2013-06-09 Thread Eric Sunshine
On Sun, Jun 9, 2013 at 2:25 AM, Nguyễn Thái Ngọc Duy  wrote:
> The function is made to use with free_pathspec() because a simple
> struct assignment is not enough (free_pathspec wants to free "items"
> pointer).

I had to read this about five or six times before I could parse and
understand it. Perhaps rewrite something like this:

free_pathspec() frees pathspec->items, hence simple struct
assignment does not properly duplicate a pathspec. Introduce
copy_pathspec() to clone a pathspec correctly by deeply copying
the items member.

> Signed-off-by: Nguyễn Thái Ngọc Duy 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html