On Wed, Apr 06, 2011 at 01:30:03AM +0200, Erik Faye-Lund wrote:
> The rather awkward return statement with strdup("") is because
> strbuf_detach returns NULL when there's nothing allocated. Even
> worse, it returns an uninitialized string if the string has been
> initialized with a non-zero 'hint'.
That seems like two bugs in the strbuf code.
I would expect strbuf_detach to _always_ return an allocated buffer,
even if it is xstrdup(""). Though the code in strbuf_detach is explicit
about returning NULL rather than slopbuf, so perhaps there is a case
where it is useful.
But for the other, one of the invariants of strbuf is that the string is
always NUL-terminated. So I would expect strbuf_init to properly
NUL-terminate after growing based on the hint.
> static char *parse_value(void)
> {
> - static char value[1024];
> - int quote = 0, comment = 0, len = 0, space = 0;
> + struct strbuf value = STRBUF_INIT;
> + int quote = 0, comment = 0, space = 0;
One thing you could about the strbuf_detach thing is to just use a
single static strbuf instead of the static array, and just reset its
length back to zero on each call. So:
static struct strbuf value = STRBUF_INIT;
...
strbuf_reset(&value);
> - value[len] = 0;
> - return value;
> + return value.len ?
> + strbuf_detach(&value, NULL) :
> + strdup("");
Then this just becomes:
return value.buf;
> default:
> + strbuf_release(&value);
> return NULL;
And you can drop the release.
> @@ -226,7 +226,9 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
> if (!value)
> return -1;
> }
> - return fn(name, value, data);
> + ret = fn(name, value, data);
> + free(value);
> + return ret;
And drop this hunk, since callers no longer need to free.
I do wonder, though, if we could be reusing the unquote_c_style()
function in quote.c. They are obviously similar, but I haven't checked
if there is more going on in the config code.
-Peff
--
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