Re: [PATCH v2 03/25] transport-helper.c: do not send null option to remote helper

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

 



Duy Nguyen <pclouds@xxxxxxxxx> writes:

> On Fri, Feb 5, 2016 at 6:22 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:
>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>>> ---
>>
>> This is even more strange.  Are the current callers broken and some
>> sends value==NULL for an option that is not is_bool, resulting in
>> a call to quote_c_style() with NULL?  I somehow find it hard to
>> believe as that would lead to an immediate segfault.
>>
>> Assuming that no current caller passes NULL to value when is_bool is
>> not in effect, there needs an explanation why future new callers may
>> need to do so.  An alternative for a valueless option could be to
>> send "option name\n" instead of the usual "option name value\n", but
>> without such an explanation, readers cannot tell why not sending
>> anything about "name", which is what this patch chooses to implement,
>> is a better idea.
>
> The source is backfill_tags() which, in future, resets some transport
> options back to defaults. The current set_option() in there only deals
> with booleans or number (depth). But in future it resets deepen-since,
> which is a string.
>
> I think the main reason is, we do not have a way to reset (or unset) a
> transport option. Should I keep this commit and explain about this, or
> have a new transport API to reset option?

Addition of new API is OK, but if this commit remains in the
history, it itself has to explain why it is necessary.
--
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]