On Sat, Apr 11, 2026 at 04:03:43PM +0000, Egor Shestakov wrote:
> Subject: [PATCH] MINOR: errors: remove excessive errmsg checks
> I noticed some strange checks for presence of errmsg. Called functions
> generate non-empty error message in case of failure, so a non-NULL address
> of the error message is enough.
>
> No backport needed.
I'm not seeing this as excessive, it just avoids emitting a message with an
empty string, even if it does not seem useful this is just a guarantee if the
message emission is not done correctly. I don't think it's useful to remove
them. Ideally we should probably wrap the error generation in some function
which would that itself everywhere... But that's another story.
> ---
> src/acme.c | 4 ++--
> src/cfgparse-listen.c | 22 +++++++++++-----------
> src/cfgparse-peers.c | 4 ++--
> src/log.c | 6 +++---
> 4 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/src/acme.c b/src/acme.c
> index eb7137b4c..437528c08 100644
> --- a/src/acme.c
> +++ b/src/acme.c
> @@ -800,8 +800,8 @@ static int cfg_postsection_acme()
> /* tries to open the account key */
> if (stat(path, &st) == 0) {
> if (ssl_sock_load_key_into_ckch(path, NULL, store->data,
> &errmsg)) {
> - memprintf(&errmsg, "%s'%s' is present but cannot be
> read or parsed.\n", errmsg && *errmsg ? errmsg : NULL, path);
> - if (errmsg && *errmsg)
> + memprintf(&errmsg, "%s'%s' is present but cannot be
> read or parsed.\n", errmsg, path);
> + if (errmsg)
> indent_msg(&errmsg, 8);
> err_code |= ERR_ALERT | ERR_FATAL | ERR_ABORT;
> ha_alert("acme: %s\n", errmsg);
In my opinion this first part should be in a separate BUG/MINOR patch which
should be backported.
--
William Lallemand