On Thu, Sep 20, 2012 at 10:43 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Junio C Hamano <gits...@pobox.com> writes:
>> Adam Spiers <g...@adamspiers.org> writes:
>>> Adam Spiers (14):
>>>   Update directory listing API doc to match code
>>>   Improve documentation and comments regarding directory traversal API
>>>   Rename cryptic 'which' variable to more consistent name
>>>   Rename path_excluded() to is_path_excluded()
>>>   Rename excluded_from_list() to is_excluded_from_list()
>>>   Rename excluded() to is_excluded()
>>>   Refactor is_excluded_from_list()
>>>   Refactor is_excluded()
>>>   Refactor is_path_excluded()
>>>   For each exclude pattern, store information about where it came from
>>>   Refactor treat_gitlinks()
>>>   Extract some useful pathspec handling code from builtin/add.c into a
>>>     library
>>>   Provide free_directory() for reclaiming dir_struct memory
>>>   Add git-check-ignore sub-command


> As to the "who owns x->src and when is it freed" question, it may
> make sense to give el a "filename" field (command line and other
> special cases would get whatever value you deem appropriate, like
> NULL or "<command line>"), have x->src point at that field when you
> queue many x's to the el in add_exc_from_file_to_list().  Then when
> you pop an element in the exclude_stack, you can free el->filename
> to plug a potential leak.

I have done this, but it required a change to struct dir:

Currently, when dir->exclude_stack is more than one entry deep, the
exclude_list pointed to by dir->exclude_list[EXC_DIRS] is a
concatenation of exclude elements from multiple files, so there will
be different values for src.  The same is true of EXC_FILE, which
typically mixes patterns from .git/info/exclude and core.excludesfile.
Therefore I have split each exclude_list into potentially multiple
exclude_lists, one per pattern source, whilst preserving the EXC_*
grouping and ordering.

My latest re-roll of as/check-ignore is nearly ready, and when I send
it, you will see a new patch in there covering the above change.

> Also I do not see why you need to have the strdup() in the caller of
> add_excludes_from_file_to_list().  If you need to keep it stable
> because you are copying it away in exclude or excludde_list,
> wouldn't it make more sense to do that at the beginning of the
> callee, i.e. add_excludes_from_file_to_list() function?

No, because in all other callers of add_excludes_from_file_to_list(),
the exclude source is already stable.  The re-roll will make this
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

Reply via email to