Re: [PATCH 02/14] pull: pass verbosity, --progress flags to fetch and merge

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 21, 2015, 15:59 UTC
Message-ID: <f90387a786c1e0f4287c9fee405f8e2f@www.dscho.org>
In-Reply-To: <CACRoPnRVcjcegRU8J6a=X6uN=b7fhJyD9=js4LqJ7ORX6gT=jg@mail.gmail.com>

Hi Paul,

On 2015-05-21 11:48, Paul Tan wrote:

Show 27 quoted lines

> 
> On Tue, May 19, 2015 at 1:41 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
>> On 2015-05-18 17:05, Paul Tan wrote:
>>> diff --git a/builtin/pull.c b/builtin/pull.c
>>> index 0b771b9..a4d9c92 100644
>>> --- a/builtin/pull.c
>>> +++ b/builtin/pull.c
>>> @@ -11,16 +11,64 @@
>>>  #include "argv-array.h"
>>>  #include "run-command.h"
>>>
>>> +/**
>>> + * Given an option opt, where opt->value points to a char* and opt->defval is a
>>> + * string, sets opt->value to the evaluation of "--$defval=$arg". If `arg` is
>>> + * NULL, then opt->value is set to "--$defval". If unset is true, then
>>> + * opt->value is set to "--no-$defval".
>>> + */
>>> +static int parse_opt_passthru(const struct option *opt, const char
>>> *arg, int unset)
>>
>> Implied by my suggested description, I also propose to put the re-recreated command-line option into a strbuf instead of a char *, to make memory management easier (read: avoid memory leaks).
> 
> Unfortunately, the usage of strbuf means that we lose the ability to
> know if an option was not provided at all (the value is NULL). This is
> important as some of these options are used to override the default
> configured behavior.

Would this not be implied by the strbuf's len being 0?

>> You might also want to verify that arg is `NULL` when `unset != 0`. Something like this:
> 
> Hmm, would there be a situation where arg is NULL when `unset != 0`?

I forgot to say that my suggestion was purely meant as defensive coding. Yes, `arg` *should* be `NULL` when `unset != 0`. Should ;-)

By the way, just to make sure: My comments and suggestions are no demands; you should feel very free to reject them when you feel that your code is better without the suggested changes. I am just trying to be helpful.

Ciao, Dscho