Re: What's cooking in git.git (Mar 2010, #06; Wed, 24)

[Alex Riesen <raa.lkml@gmail.com>]

On Thu, Mar 25, 2010 at 17:27, Brandon Casey
<brandon.casey.ctr@nrlssc.navy.mil> wrote:
> On 03/25/2010 10:11 AM, Nguyen Thai Ngoc Duy wrote:
>> 2010/3/25 Junio C Hamano <gitster@pobox.com>:
>>> * ar/config-from-command-line (2010-03-19) 1 commit
>>>  - Allow passing of configuration parameters in the command line
>> This breaks my build on Solaris because it uses strndup, which is not available.
>
> A quick glance at ar/config-from.. also detected an unchecked calloc().
>
> Alex, any reason xcalloc wasn't used?

Just missed it. Long time away from Git sources.

> btw, me also thinks the code is a little hard to read.  For example, I initially
> thought your calloc was not allocating enough space for the nul terminator.
>
>   ct = calloc(1, sizeof(struct config_item) + (text - name));
>   memcpy(ct->name, name, text - name);
>
> I traced the code, but it wasn't until I noticed that your data structure looks
> like this:
>
>   struct config_item
>   {
>          struct config_item *next;
>          char *value;
>          char name[1];
>   };
>
> that I realized that room for the nul terminator in the 'name' array was allocated
> by the structure itself, since the name declaration looks like name[1] and not
> name[FLEX_ARRAY].

Hmm, I kinda liked how the space for the terminator was reserved and
all the flex array incompatibilities were worked around.

> Would the code be simpler if strbufs were used?  Then you wouldn't need to duplicate
> the skip_space and trailing_space functionality provided in the strbuf library, and
> would just need a new function named strbuf_tolower.

But this indeed makes sense. Promise to take a look at it after some sleep.

> Also, should config_parametes_tail be spelled config_parameters_tail?

Yep.

Thanks!