Re: [PATCH 01/41] usage.c: move format processing out of die_errno()

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 1, 2016, 18:23 UTC
Message-ID: <CAPig+cQhQ2C-gOuQwJ9RBXM7HBBJkWORfRkq-t-PY=vvxgAonA@mail.gmail.com>
In-Reply-To: <1462101297-8610-2-git-send-email-pclouds@gmail.com>

On Sun, May 1, 2016 at 7:14 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

Show 13 quoted lines

> fmt_with_err() will be shared with the coming error_errno() and
> warning_errno().
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/usage.c b/usage.c
> @@ -109,19 +109,12 @@ void NORETURN die(const char *err, ...)
> -void NORETURN die_errno(const char *fmt, ...)
> +static const char *fmt_with_err(const char *fmt)
>  {
> -       va_list params;
> -       char fmt_with_err[1024];
> +       static char fmt_with_err[1024];

Rather than this static buffer, did you consider having the caller pass in the buffer?

    static const char *fmt_with_err(char *buf, size_t n, const char *fmt)
    {
        ...
        snprintf(buf, n, "%s: %s", fmt, str_error);
        return buf;
    }

    void die_errno(const char *fmt, ...)
    {
        char fmtbuf[1024];
        ...
        die_routine(fmt_with_err(fmtbuf, sizeof(fmtbuf), fmt),
            params);
        ...
    }

Better? Worse? Indifferent?

Show 34 quoted lines

>         char str_error[256], *err;
>         int i, j;
>
> -       if (die_is_recursing()) {
> -               fputs("fatal: recursion detected in die_errno handler\n",
> -                       stderr);
> -               exit(128);
> -       }
> -
>         err = strerror(errno);
>         for (i = j = 0; err[i] && j < sizeof(str_error) - 1; ) {
>                 if ((str_error[j++] = err[i++]) != '%')
> @@ -137,9 +130,21 @@ void NORETURN die_errno(const char *fmt, ...)
>         }
>         str_error[j] = 0;
>         snprintf(fmt_with_err, sizeof(fmt_with_err), "%s: %s", fmt, str_error);
> +       return fmt_with_err;
> +}
> +
> +void NORETURN die_errno(const char *fmt, ...)
> +{
> +       va_list params;
> +
> +       if (die_is_recursing()) {
> +               fputs("fatal: recursion detected in die_errno handler\n",
> +                       stderr);
> +               exit(128);
> +       }
>
>         va_start(params, fmt);
> -       die_routine(fmt_with_err, params);
> +       die_routine(fmt_with_err(fmt), params);
>         va_end(params);
>  }