Re: [RFC/PATCH V2] alias.c: replace git_config with git_config_get_string

> On Mon, Jun 23, 2014 at 6:41 AM, Tanay Abhra <tanayabh@gmail.com> wrote:
>> Use git_config_get_string instead of git_config to take advantage of
>> the config hash-table api which provides a cleaner control flow.
>>
>> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>
>> ---
>>  alias.c | 28 ++++++++++------------------
>>  1 file changed, 10 insertions(+), 18 deletions(-)
>>
>> diff --git a/alias.c b/alias.c
>> index 5efc3d6..0fe32bc 100644
>> --- a/alias.c
>> +++ b/alias.c
>> @@ -1,25 +1,17 @@
>>  #include "cache.h"
>>
>> -static const char *alias_key;
>> -static char *alias_val;
>> -
>> -static int alias_lookup_cb(const char *k, const char *v, void *cb)
>> -{
>> -       if (starts_with(k, "alias.") && !strcmp(k + 6, alias_key)) {
>> -               if (!v)
>> -                       return config_error_nonbool(k);
>> -               alias_val = xstrdup(v);
>> -               return 0;
>> -       }
>> -       return 0;
>> -}
>> -
>>  char *alias_lookup(const char *alias)
>>  {
>> -       alias_key = alias;
>> -       alias_val = NULL;
>> -       git_config(alias_lookup_cb, NULL);
>> -       return alias_val;
>> +       const char *v;
>> +       char *value;
>> +       struct strbuf key = STRBUF_INIT;
>> +       strbuf_addf(&key, "alias.%s", alias);
>> +       git_config_get_string(key.buf, &v);
>> +       if (!v)
>> +               config_error_nonbool(key.buf);
> 
> If 'v' is NULL, you correctly report an error, but then fall through
> and invoke xstrdup() with NULL, which invites undefined behavior [1].
> 
> [1]: http://pubs.opengroup.org/onlinepubs/009695399/functions/strdup.html
> 
>> +       value = xstrdup(v);
>> +       strbuf_release(&key);
>> +       return value;
> 
> You could release the strbuf earlier, which would allow you to 'return
> xstrdup(v)' and drop the 'value' variable. Perhaps you want something
> like this:
> 
>     const char *v;
>     struct strbuf key = STRBUF_INIT;
>     strbuf_addf(&key, "alias.%s", alias);
>     git_config_get_string(key.buf, &v);
>     if (v)
>         config_error_nonbool(key.buf);
>     strbuf_release(&key);
>     return v ? xstrdup(v) : NULL;
> 

>>  }
>>
>>  #define SPLIT_CMDLINE_BAD_ENDING 1
>> --
>> 1.9.0.GIT
>