Re: [PATCH v2 00/14] new git check-ignore sub-command
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 20, 2012, 21:43 UTC
- Message-ID
- <7vsjac8j52.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <7v4nms9yja.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 22 quoted lines
> Adam Spiers <git@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 > > Please retitle these to have a short "prefix: " that names a > specific area the series intends to touch. I retitled your other > series to share "test :" as their common prefix.
Just to clarify, I think most of them can say "dir.c: ".
I saw quite a few decl-after-statement in new code. Please fix them.
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.
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?