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

From: Duy Nguyen <pclouds@gmail.com>
Date: Feb 6, 2016, 09:38 UTC
Message-ID: <CACsJy8Az9rx_D0V+p2fEdNi64YJb2g_o0npUk2=wqCL2_moBPQ@mail.gmail.com>
In-Reply-To: <xmqqh9hof4bx.fsf@gitster.mtv.corp.google.com>

On Fri, Feb 5, 2016 at 6:22 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>
> 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?

-- 
Duy