On 2023-Sep-15, Daniel Gustafsson wrote: > -basic_archive_configured(ArchiveModuleState *state) > +basic_archive_configured(ArchiveModuleState *state, const char **errmsg) > > The variable name errmsg implies that it will contain the errmsg() data when > it > in fact is used for errhint() data, so it should be named accordingly. > > It's probably better to define the interface as ArchiveCheckConfiguredCB > functions returning an allocated string in the passed pointer which the caller > is responsible for freeing.
Also note that this callback is documented in archive-modules.sgml, so that needs to be updated as well. This also means you can't backpatch this change, or you risk breaking external software that implements this interface. I suggest that 'msg' shouldn't be a global variable. There's no need for that AFAICS; but if there is, this is a terrible name for it. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/