[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