Re: [PATCH v2 00/14] new git check-ignore sub-command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Adam Spiers <git@xxxxxxxxxxxxxx> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]