{"thread":{"id":"41522","subject":"[PATCH] run-command: fix an 'different modifiers' sparse warning","startedAt":"2016-02-25T19:35:07Z","lastAt":"2016-02-25T23:08:31Z","messageCount":4,"participants":["Ramsay Jones","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"279371","messageId":"56CF576B.4020401@ramsayjones.plus.com","threadId":"41522","inReplyTo":null,"subject":"[PATCH] run-command: fix an 'different modifiers' sparse warning","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-02-25T19:35:07Z","receivedAt":"2016-02-25T19:35:07Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSigned-off-by: Ramsay Jones \n---\n\nHi Jeff,\n\nIf you need to re-roll your 'jk/epipe-in-async' branch, could you\nplease squash this into the relevant patch. (ie. \"write_or_die:\nhandle EPIPE in async threads\", 24-02-2016).\n\nThanks!\n\nATB,\nRamsay Jones\n\n run-command.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex cd861bc..5dec18b 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -625,7 +625,7 @@ int in_async(void)\n \treturn !pthread_equal(main_thread, pthread_self());\n }\n \n-void async_exit(int code)\n+void NORETURN async_exit(int code)\n {\n \tpthread_exit((void *)(intptr_t)code);\n }\n@@ -675,7 +675,7 @@ int in_async(void)\n \treturn process_is_async;\n }\n \n-int async_exit(int code)\n+int NORETURN async_exit(int code)\n {\n \texit(code);\n }\n-- \n2.7.0\n"},{"id":"279377","messageId":"xmqqfuwgo83n.fsf@gitster.mtv.corp.google.com","threadId":"41522","inReplyTo":"56CF576B.4020401@ramsayjones.plus.com","subject":"Re: [PATCH] run-command: fix an 'different modifiers' sparse warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T20:20:12Z","receivedAt":"2016-02-25T20:20:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> Signed-off-by: Ramsay Jones \n> ---\n>\n> Hi Jeff,\n>\n> If you need to re-roll your 'jk/epipe-in-async' branch, could you\n> please squash this into the relevant patch. (ie. \"write_or_die:\n> handle EPIPE in async threads\", 24-02-2016).\n>\n> Thanks!\n\nI actually was planning to merge this to 'next' today, so I'll\nsquash it in without waiting for a reroll.\n\nBy the way, doesn't it bother anybody to give two different types to\nthe same function depending on NO_PTHREAD? It is not a new issue\nadded by this series, but async_exit() that claims to return int\ndoes not (naturally) return anything, and sparse does not seem to\ncare (neither do we).\n\n>\n> ATB,\n> Ramsay Jones\n>\n> run-command.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/run-command.c b/run-command.c\n> index cd861bc..5dec18b 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -625,7 +625,7 @@ int in_async(void)\n> \treturn !pthread_equal(main_thread, pthread_self());\n> }\n> \n> -void async_exit(int code)\n> +void NORETURN async_exit(int code)\n> {\n> \tpthread_exit((void *)(intptr_t)code);\n> }\n> @@ -675,7 +675,7 @@ int in_async(void)\n> \treturn process_is_async;\n> }\n> \n> -int async_exit(int code)\n> +int NORETURN async_exit(int code)\n> {\n> \texit(code);\n> }\n"},{"id":"279388","messageId":"20160225213935.GA3612@sigill.intra.peff.net","threadId":"41522","inReplyTo":"xmqqfuwgo83n.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] run-command: fix an 'different modifiers' sparse warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-25T21:39:35Z","receivedAt":"2016-02-25T21:39:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 25, 2016 at 12:20:12PM -0800, Junio C Hamano wrote:\n\n> Ramsay Jones writes:\n> \n> > Signed-off-by: Ramsay Jones \n> > ---\n> >\n> > Hi Jeff,\n> >\n> > If you need to re-roll your 'jk/epipe-in-async' branch, could you\n> > please squash this into the relevant patch. (ie. \"write_or_die:\n> > handle EPIPE in async threads\", 24-02-2016).\n> >\n> > Thanks!\n> \n> I actually was planning to merge this to 'next' today, so I'll\n> squash it in without waiting for a reroll.\n\nI am OK with that. But I do find it interesting that we must mark\nNORETURN in both the declaration and the definition, but we don't for\n__attribute__((format)).\n\n> By the way, doesn't it bother anybody to give two different types to\n> the same function depending on NO_PTHREAD? It is not a new issue\n> added by this series, but async_exit() that claims to return int\n> does not (naturally) return anything, and sparse does not seem to\n> care (neither do we).\n\nIt would have bothered me if I had noticed. :)\n\nIt is simply a bug, and sparse (and the compiler) do not notice it\nbecause it only shows up if you compile with NO_PTHREADS=1. And I think\nit is added by this series:\n\n> > @@ -675,7 +675,7 @@ int in_async(void)\n> > \treturn process_is_async;\n> > }\n> > \n> > -int async_exit(int code)\n> > +int NORETURN async_exit(int code)\n\nThe return value on this one should be \"void\", too, of course.\n\n-Peff\n"},{"id":"279419","messageId":"56CF896F.3000601@ramsayjones.plus.com","threadId":"41522","inReplyTo":"20160225213935.GA3612@sigill.intra.peff.net","subject":"Re: [PATCH] run-command: fix an 'different modifiers' sparse warning","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-02-25T23:08:31Z","receivedAt":"2016-02-25T23:08:31Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 25/02/16 21:39, Jeff King wrote:\n> On Thu, Feb 25, 2016 at 12:20:12PM -0800, Junio C Hamano wrote:\n> \n>> Ramsay Jones writes:\n>>\n>>> Signed-off-by: Ramsay Jones \n>>> ---\n>>>\n>>> Hi Jeff,\n>>>\n>>> If you need to re-roll your 'jk/epipe-in-async' branch, could you\n>>> please squash this into the relevant patch. (ie. \"write_or_die:\n>>> handle EPIPE in async threads\", 24-02-2016).\n>>>\n>>> Thanks!\n>>\n>> I actually was planning to merge this to 'next' today, so I'll\n>> squash it in without waiting for a reroll.\n> \n> I am OK with that. But I do find it interesting that we must mark\n> NORETURN in both the declaration and the definition, but we don't for\n> __attribute__((format)).\n> \n>> By the way, doesn't it bother anybody to give two different types to\n>> the same function depending on NO_PTHREAD? It is not a new issue\n>> added by this series, but async_exit() that claims to return int\n>> does not (naturally) return anything, and sparse does not seem to\n>> care (neither do we).\n> \n> It would have bothered me if I had noticed. :)\n> \n> It is simply a bug, and sparse (and the compiler) do not notice it\n> because it only shows up if you compile with NO_PTHREADS=1. And I think\n> it is added by this series:\n\nWow, I can't believe that I didn't notice this myself while editing\nthe file! :-D\n\nATB,\nRamsay Jones\n"}]}