{"thread":{"id":"51065","subject":"[PATCH] http-push: workaround for format overflow warning in gcc >= 9","startedAt":"2019-05-10T06:59:04Z","lastAt":"2019-05-13T05:57:51Z","messageCount":3,"participants":["Carlo Marcelo Arenas Belón","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"375281","messageId":"20190510065848.27606-1-carenas@gmail.com","threadId":"51065","inReplyTo":null,"subject":"[PATCH] http-push: workaround for format overflow warning in gcc >= 9","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2019-05-10T06:58:48Z","receivedAt":"2019-05-10T06:59:04Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"In function 'finish_request',\n    inlined from 'process_response' at http-push.c:248:2:\nhttp-push.c:587:4: warning: '%s' directive argument is null [-Wformat-overflow=]\n  587 |    fprintf(stderr, \"Unable to get pack file %s\\n%s\",\n      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n  588 |     request->url, curl_errorstr);\n      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n---\n http-push.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex f675a96316..2db553ef38 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -585,7 +585,8 @@ static void finish_request(struct transfer_request *request)\n \t\tint fail = 1;\n \t\tif (request->curl_result != CURLE_OK) {\n \t\t\tfprintf(stderr, \"Unable to get pack file %s\\n%s\",\n-\t\t\t\trequest->url, curl_errorstr);\n+\t\t\t\trequest->url ? request->url : \"\",\n+\t\t\t\tcurl_errorstr);\n \t\t} else {\n \t\t\tpreq = (struct http_pack_request *)request->userData;\n \n-- \n2.21.0\n\n"},{"id":"375282","messageId":"CAPig+cSY4SKXcppM6UYpeJ3z+PrUwiJ=whCmHQv4e_3CUZSwoQ@mail.gmail.com","threadId":"51065","inReplyTo":"20190510065848.27606-1-carenas@gmail.com","subject":"Re: [PATCH] http-push: workaround for format overflow warning in gcc >= 9","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-10T07:50:32Z","receivedAt":"2019-05-10T07:50:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 10, 2019 at 2:59 AM Carlo Marcelo Arenas Belón\n<carenas@gmail.com> wrote:\n> In function 'finish_request',\n>     inlined from 'process_response' at http-push.c:248:2:\n> http-push.c:587:4: warning: '%s' directive argument is null [-Wformat-overflow=]\n>   587 |    fprintf(stderr, \"Unable to get pack file %s\\n%s\",\n>       |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n>   588 |     request->url, curl_errorstr);\n>       |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> ---\n\nMissing sign-off.\n\n> diff --git a/http-push.c b/http-push.c\n> @@ -585,7 +585,8 @@ static void finish_request(struct transfer_request *request)\n>                 int fail = 1;\n>                 if (request->curl_result != CURLE_OK) {\n>                         fprintf(stderr, \"Unable to get pack file %s\\n%s\",\n> -                               request->url, curl_errorstr);\n> +                               request->url ? request->url : \"\",\n> +                               curl_errorstr);\n>                 } else {\n\nIf I'm reading the code correctly, the conditional and \"true\" branch\nof the ternary expression are dead code since 'request->url' will\nunconditionally be NULL due to the:\n\n    /* URL is reused for MOVE after PUT */\n    if (request->state != RUN_PUT) {\n        FREE_AND_NULL(request->url);\n    }\n\nearlier in the function. If you want to present a meaningful error\nmessage here, I could imagine squirreling-away the URL so it can be\nused in the error message, or re-working the code so that\nFREE_AND_NULL(request->url) is only done when and if needed.\n"},{"id":"375365","messageId":"xmqqa7fqdhgl.fsf@gitster-ct.c.googlers.com","threadId":"51065","inReplyTo":"CAPig+cSY4SKXcppM6UYpeJ3z+PrUwiJ=whCmHQv4e_3CUZSwoQ@mail.gmail.com","subject":"Re: [PATCH] http-push: workaround for format overflow warning in gcc >= 9","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-13T05:57:46Z","receivedAt":"2019-05-13T05:57:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>>                 if (request->curl_result != CURLE_OK) {\n>>                         fprintf(stderr, \"Unable to get pack file %s\\n%s\",\n>> -                               request->url, curl_errorstr);\n>> +                               request->url ? request->url : \"\",\n>> +                               curl_errorstr);\n>>                 } else {\n>\n> If I'm reading the code correctly, the conditional and \"true\" branch\n> of the ternary expression are dead code since 'request->url' will\n> unconditionally be NULL due to the:\n>\n>     /* URL is reused for MOVE after PUT */\n>     if (request->state != RUN_PUT) {\n>         FREE_AND_NULL(request->url);\n>     }\n>\n> earlier in the function. If you want to present a meaningful error\n> message here, I could imagine squirreling-away the URL so it can be\n> used in the error message, or re-working the code so that\n> FREE_AND_NULL(request->url) is only done when and if needed.\n\nThat matches my understanding.\n"}]}