On 19/08/2025 23:49, Jelte Fennema-Nio wrote:
On Thu, 14 Aug 2025 at 15:10, Heikki Linnakangas <hlinn...@iki.fi> wrote:
Here's a new set of patches, to disconnect on OOM instead of hanging or
silently discarding messages:
Code looks good. Som small nitpicks though.
This change seems unnecessary, i.e. free(NULL) is a no-op
- free(svname);
+ if (svname)
+ free(svname);
And even more so, this is unreachable when svname == NULL. Thanks!
Small wording suggestion, maybe change this:
The caller has already saved the error message in conn->errorMessage.
to
The caller should have already saved the error message in conn->errorMessage.
or even
The caller should have already saved the error message using
libpq_append_conn_error.
I kept it as is, because we use similar wording in a few other places.
Some places do write the message directly in conn->errorMessage without
using libpq_append_conn_error.
Pushed and backpatched to v18. I feel the OOM handling commit would be
appropriate to backpatch further, but since it's pretty intricate code
and we haven't gotten any complaints from the field, I only backpatched
it to v18 for now. We can backpatch it further later if needed.
Thanks for the original patch and the review!
- Heikki
