> On Feb 12, 2026, at 15:43, Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com> wrote:
>
> On Wed, Feb 11, 2026 at 1:16 PM Mircea Cadariu <cadariu.mircea@gmail.com> 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
parameterultimately 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()sstate->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/
