On Sun, Apr 24, 2016 at 4:50 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote: > On 6 January 2016 at 19:56, Frank Henigman <fjhenig...@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. _______________________________________________ waffle mailing list waffle@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/waffle