threads / patch / 41522

patchrun-command: fix an 'different modifiers' sparse warning

Subject: [PATCH] run-command: fix an 'different modifiers' sparse warning

## tl;dr

4 messages between Feb 25, 2016 and Feb 25, 2016. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Ramsay Jones· Feb 25, 2016, 19:35 UTC · lore

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Jeff,

If you need to re-roll your 'jk/epipe-in-async' branch, could you please squash this into the relevant patch. (ie. "write_or_die: handle EPIPE in async threads", 24-02-2016).

Thanks!

ATB, Ramsay Jones

 run-command.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Show changes to run-command.c +2 −2

diff --git a/run-command.c b/run-command.c
index cd861bc..5dec18b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -625,7 +625,7 @@ int in_async(void)
 	return !pthread_equal(main_thread, pthread_self());
 }
 
-void async_exit(int code)
+void NORETURN async_exit(int code)
 {
 	pthread_exit((void *)(intptr_t)code);
 }
@@ -675,7 +675,7 @@ int in_async(void)
 	return process_is_async;
 }
 
-int async_exit(int code)
+int NORETURN async_exit(int code)
 {
 	exit(code);
 }

-- 
2.7.0

Junio C Hamano· Feb 25, 2016, 20:20 UTC · re: Ramsay Jones · lore

Re: [PATCH] run-command: fix an 'different modifiers' sparse warning

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

Show 10 quoted lines

> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>
> Hi Jeff,
>
> If you need to re-roll your 'jk/epipe-in-async' branch, could you
> please squash this into the relevant patch. (ie. "write_or_die:
> handle EPIPE in async threads", 24-02-2016).
>
> Thanks!

I actually was planning to merge this to 'next' today, so I'll squash it in without waiting for a reroll.

By the way, doesn't it bother anybody to give two different types to the same function depending on NO_PTHREAD? It is not a new issue added by this series, but async_exit() that claims to return int does not (naturally) return anything, and sparse does not seem to care (neither do we).

Show 29 quoted lines

>
> ATB,
> Ramsay Jones
>
>  run-command.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/run-command.c b/run-command.c
> index cd861bc..5dec18b 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -625,7 +625,7 @@ int in_async(void)
>  	return !pthread_equal(main_thread, pthread_self());
>  }
>  
> -void async_exit(int code)
> +void NORETURN async_exit(int code)
>  {
>  	pthread_exit((void *)(intptr_t)code);
>  }
> @@ -675,7 +675,7 @@ int in_async(void)
>  	return process_is_async;
>  }
>  
> -int async_exit(int code)
> +int NORETURN async_exit(int code)
>  {
>  	exit(code);
>  }

Jeff King· Feb 25, 2016, 21:39 UTC · re: Junio C Hamano · lore

Re: [PATCH] run-command: fix an 'different modifiers' sparse warning

On Thu, Feb 25, 2016 at 12:20:12PM -0800, Junio C Hamano wrote:

Show 15 quoted lines

> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
> > Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> > ---
> >
> > Hi Jeff,
> >
> > If you need to re-roll your 'jk/epipe-in-async' branch, could you
> > please squash this into the relevant patch. (ie. "write_or_die:
> > handle EPIPE in async threads", 24-02-2016).
> >
> > Thanks!
> 
> I actually was planning to merge this to 'next' today, so I'll
> squash it in without waiting for a reroll.

I am OK with that. But I do find it interesting that we must mark NORETURN in both the declaration and the definition, but we don't for __attribute__((format)).

Show 5 quoted lines

> By the way, doesn't it bother anybody to give two different types to
> the same function depending on NO_PTHREAD?  It is not a new issue
> added by this series, but async_exit() that claims to return int
> does not (naturally) return anything, and sparse does not seem to
> care (neither do we).

It would have bothered me if I had noticed. :)

It is simply a bug, and sparse (and the compiler) do not notice it because it only shows up if you compile with NO_PTHREADS=1. And I think it is added by this series:

Show 6 quoted lines

> > @@ -675,7 +675,7 @@ int in_async(void)
> >  	return process_is_async;
> >  }
> >  
> > -int async_exit(int code)
> > +int NORETURN async_exit(int code)

The return value on this one should be "void", too, of course.

-Peff

Ramsay Jones· Feb 25, 2016, 23:08 UTC · re: Jeff King · lore

Re: [PATCH] run-command: fix an 'different modifiers' sparse warning

On 25/02/16 21:39, Jeff King wrote:

Show 33 quoted lines

> On Thu, Feb 25, 2016 at 12:20:12PM -0800, Junio C Hamano wrote:
> 
>> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>>
>>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>>> ---
>>>
>>> Hi Jeff,
>>>
>>> If you need to re-roll your 'jk/epipe-in-async' branch, could you
>>> please squash this into the relevant patch. (ie. "write_or_die:
>>> handle EPIPE in async threads", 24-02-2016).
>>>
>>> Thanks!
>>
>> I actually was planning to merge this to 'next' today, so I'll
>> squash it in without waiting for a reroll.
> 
> I am OK with that. But I do find it interesting that we must mark
> NORETURN in both the declaration and the definition, but we don't for
> __attribute__((format)).
> 
>> By the way, doesn't it bother anybody to give two different types to
>> the same function depending on NO_PTHREAD?  It is not a new issue
>> added by this series, but async_exit() that claims to return int
>> does not (naturally) return anything, and sparse does not seem to
>> care (neither do we).
> 
> It would have bothered me if I had noticed. :)
> 
> It is simply a bug, and sparse (and the compiler) do not notice it
> because it only shows up if you compile with NO_PTHREADS=1. And I think
> it is added by this series:

Wow, I can't believe that I didn't notice this myself while editing the file! :-D

ATB, Ramsay Jones