[waffle] [PATCH 04/12] core: add JSON library

Frank Henigman fjhenigman at google.com
Mon Apr 25 03:14:47 UTC 2016


On Sun, Apr 24, 2016 at 4:50 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 6 January 2016 at 19:56, Frank Henigman <fjhenigman at google.com> wrote:
>
>> +static void
>> +put(struct json *jj, char *s)
>> +{
>> +    if (!jj->buf)
>> +        return;
>> +
>> +    for (;;) {
>> +        if (!(*jj->pos = *s++))
>> +            break;
>> +        if (++jj->pos == jj->buf + jj->size) {
>> +            size_t z = jj->size * 2;
>> +            jj->buf = realloc(jj->buf, z);
>> +            if (!jj->buf)
>
> As I did not look closely at the patch this piece made me believe it
> will corrupt the output. Although it will only leak memory.
> Namely: if realloc fails, the old pointer is lost yet the data isn't freed.

Nice catch.  When I did the testing with simulated allocation failures
I also checked for leaks, but I probably implemented the realloc
wrapper wrong (probably unconditionally freed the given pointer).  I
did the testing again and it found this leak.  Easily fixed by doing a
free when the realloc fails.  So I'll go out on a limb and claim v2
will not crash, leak or return a partial result if memory runs out at
any point in the building of a json string.

>
>> +    char *buf = malloc(strlen(s) * 3 + 2);
>> +    if (!buf)
> Nit: Just return NULL and drop the label ? Imho there is no point in
> jumping only to free(NULL).

It jumps to free(dup) where dup!=NULL.

> Thanks
> Emil

Emil: sorry for the duplicate - I didn't reply to the list the first time.


More information about the waffle mailing list