Fix realloc memory leak

[lazydroid]

realloc() may return NULL, so when we do:

foobar = realloc( foobar );

we risk leaking memory, better implementation would be:

maybe_good = realloc( foobar )
foobar = maybe_good != NULL ? maybe_good : foobar
....
free( foobar )

[lazydroid]

the change did not seem to break anything important

[rschlaikjer]

Seems fine. Should it attempt to still log however many bytes were written to the buffer before it failed to realloc to try and eke out potentially useful logs in an OOM situation? If realloc actually returns nullptr all bets are off though. Overcommit means that it's unlikely to ever actually happen (allocation will succeed, program will just trap on access).

[lazydroid]

log however many bytes were written to the buffer before it failed

I'm not sure there's anything guaranteed to be written if there's not enough space: "A negative number is returned on failure, including when the resulting string to be written to ws would be longer than n characters."

[rschlaikjer]

Nothing is guaranteed, but the implementation in glibc writes directly to the buffer as it goes, so you will likely have some large fraction of wslen bytes holding valid formatted data (assuming this is not the first call to realloc, in which case the valid data is in the stacked buffer, but same idea). Sticking a \0 on the end of the buffer in the failure case and returning wslen bytes written would attempt to print as much as it could, with some amount of junk on the end.

This is mostly an idle thought - I think we are unlikely to hit this codepath regardless.

[rdiankov]

I'm confused here. Don't you have to change wslen back to the original value? Otherwise vswprintf will happily write beyond the buffer, and now we have memory corruption.

[lazydroid]

When we get a NULL, wr becomes -1 and we get straight to this part (skipping printing):

        if (wsallocated != NULL) { \
            free(wsallocated); \
            wsallocated = NULL; \
        } \