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

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

[Carlo Marcelo Arenas Belón]

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);
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
---
 http-push.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/http-push.c b/http-push.c
index f675a96316..2db553ef38 100644
--- 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 {
 			preq = (struct http_pack_request *)request->userData;
 
-- 
2.21.0

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

[Eric Sunshine]

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

> 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.

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

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>>                 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.

That matches my understanding.