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

From: Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil>
Date: Mar 25, 2010, 16:27 UTC
Message-ID: <25yNvbuAivZlBuEJoRhXtfoEc7HEZorM5qOrwgQKk7FER_fdzljyMQ@cipher.nrlssc.navy.mil>
In-Reply-To: <fcaeb9bf1003250811g777b2881l332e6f61456d699@mail.gmail.com>

On 03/25/2010 10:11 AM, Nguyen Thai Ngoc Duy wrote:

Show 8 quoted lines

> 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
>>
>> * bc/t5505-fix (2010-03-19) 3 commits
>>  - t/t5505-remote.sh: escape * to prevent interpretation by shell as glob
>>  - t5505: add missing &&
>>  - t5505: remove unnecessary subshell invocations

methinks you were over-aggressive with your cut/paste? I don't think bc/t5505-fix contains a strndup. Hopefully, it does not break your build on Solaris.

> 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?

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].

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.

Also, should config_parametes_tail be spelled config_parameters_tail?

-brandon