Re: [PATCH] http-push: workaround for format overflow warning in gcc >= 9

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 10, 2019, 07:50 UTC
Message-ID: <CAPig+cSY4SKXcppM6UYpeJ3z+PrUwiJ=whCmHQv4e_3CUZSwoQ@mail.gmail.com>
In-Reply-To: <20190510065848.27606-1-carenas@gmail.com>

On Fri, May 10, 2019 at 2:59 AM Carlo Marcelo Arenas Belón <carenas@gmail.com> wrote:

Show 8 quoted lines

> In function 'finish_request',
>     inlined from 'process_response' at http-push.c:248:2:
> http-push.c:587:4: warning: '%s' directive argument is null [-Wformat-overflow=]
>   587 |    fprintf(stderr, "Unable to get pack file %s\n%s",
>       |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   588 |     request->url, curl_errorstr);
>       |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ---

Missing sign-off.

Show 9 quoted lines

> diff --git a/http-push.c b/http-push.c
> @@ -585,7 +585,8 @@ static void finish_request(struct transfer_request *request)
>                 int fail = 1;
>                 if (request->curl_result != CURLE_OK) {
>                         fprintf(stderr, "Unable to get pack file %s\n%s",
> -                               request->url, curl_errorstr);
> +                               request->url ? request->url : "",
> +                               curl_errorstr);
>                 } else {

If I'm reading the code correctly, the conditional and "true" branch of the ternary expression are dead code since 'request->url' will unconditionally be NULL due to the:

    /* URL is reused for MOVE after PUT */
    if (request->state != RUN_PUT) {
        FREE_AND_NULL(request->url);
    }

earlier in the function. If you want to present a meaningful error message here, I could imagine squirreling-away the URL so it can be used in the error message, or re-working the code so that FREE_AND_NULL(request->url) is only done when and if needed.