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

From: Jeff King <peff@peff.net>
Date: Feb 25, 2016, 21:39 UTC
Message-ID: <20160225213935.GA3612@sigill.intra.peff.net>
In-Reply-To: <xmqqfuwgo83n.fsf@gitster.mtv.corp.google.com>

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