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


Reply via email to