From: Alex Riesen Date: Thu, 25 Mar 2010 23:11:51 GMT Subject: Re: What's cooking in git.git (Mar 2010, #06; Wed, 24) Message-ID: <81b0412b1003251611h62222eabw247ec59678c198ad@mail.gmail.com> In-Reply-To: <25yNvbuAivZlBuEJoRhXtfoEc7HEZorM5qOrwgQKk7FER_fdzljyMQ@cipher.nrlssc.navy.mil> On Thu, Mar 25, 2010 at 17:27, Brandon Casey wrote: > On 03/25/2010 10:11 AM, Nguyen Thai Ngoc Duy wrote: >> 2010/3/25 Junio C Hamano : >>> * 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!