> On Feb 12, 2026, at 15:43, Anthonin Bonnefoy
> <[email protected]> wrote:
>
> On Wed, Feb 11, 2026 at 1:16 PM Mircea Cadariu <[email protected]>
> wrote:
>> Indeed. Should we place this initialisation at the top of
>> XLogFindNextRecord, before any processing? At that point, there's
>> nothing to erase.
>
> That makes sense, I've added the '*errormsg = NULL' at the top of the
> function.
>
>> You can consider capitalising.
>
> Done
> <v5-0001-Propagate-errormsg-to-XLogFindNextRecord-caller.patch>
Hi Anthonin,
Thanks for the patch. I agree it’s useful to print a more detailed error
message instead of the generic one.
From a design perspective, I’m not sure we need to add a new errormsg parameter
to XLogFindNextRecord(). The new parameter ultimately just exposes
state->errormsg_buf, so the returned errormsg implicitly depends on the
lifetime of state, and we also need extra handling for cases like errormsg ==
NULL.
Instead, perhaps we could add a helper function, say
XLogReaderGetLastError(XLogReaderState *state). which internally pstrdup()s
state->errormsg_buf (after checking errormsg_deferred, etc.). That way the
caller owns the returned string explicitly, and there’s no hidden dependency on
the reader state’s lifetime.
This would also avoid changing the XLogFindNextRecord() signature while making
the ownership semantics clearer.
What do you think?
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
