Subject: Re: [PATCH] defaults for where to merge from (take 3, inline)

From: Jakub Narebski <[email-masked]>
Date: Fri, 02 Mar 2007 15:10:09 +0100

Johannes Schindelin wrote:
>On Thu, 1 Mar 2007, Paolo Bonzini wrote:

>>  
>> +static void register_branch_pull (const char *name, const char
*remote_name)
> 
> It is not yet remote_name, right? it is branch_name. You extract the 
> remote_name by finding the first slash.

I'm bit reluctant about this, as it is not required that remote
names cannot contain slashes. But remotes with slashes would make
separation into remote and tracking branch part more difficult.

>> +{
>> +     char *slash = strchr(remote_name, '/');
>> +
>> +     char *config_key = xmalloc(strlen(name) + 15);
>> +     char *merge_value = xmalloc(strlen(remote_name) + 10);
>> +
>> +     char *remote_value = xstrdup(remote_name);
> 
> I'd use "char key[1024], value[1024]" instead, erroring out if one of the 
> buffers are too small. It's not like you have to be memory efficient, and 
> it is easier to read.

The config parser has lengths limits on fully qualified key name (with
section name and subsection name) _and_ on value name. Use them, please,
instead of dynamic allocation and troubles with those.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git