Re: BackendKeyData is mandatory?

On 19/08/2025 23:49, Jelte Fennema-Nio wrote:
> On Thu, 14 Aug 2025 at 15:10, Heikki Linnakangas <hlinnaka@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