On 03/25/2010 10:11 AM, Nguyen Thai Ngoc Duy wrote:
> 2010/3/25 Junio C Hamano <gitster@xxxxxxxxx>:
>> * 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html