Subject: Re: [PATCH v3 0/4] remove extern from function declarations

From: Denton Liu <[email-masked]>
Date: Thu, 25 Apr 2019 11:05:12 -0700

On Thu, Apr 25, 2019 at 02:07:58PM +0200, SZEDER Gábor wrote:
> On Mon, Apr 22, 2019 at 05:49:01PM -0400, Jeff King wrote:
> > On Wed, Apr 17, 2019 at 12:58:31AM -0700, Denton Liu wrote:
>  
> > >  compat/mingw.c                    |   2 +-
> > >  compat/mingw.h                    |   6 +-
> > >  compat/nedmalloc/malloc.c.h       |   6 +-
> > >  compat/obstack.h                  |  14 +-
> > >  compat/poll/poll.h                |   2 +-
> > >  compat/regex/regex.h              |  66 ++---
> > >  compat/win32/pthread.h            |   8 +-
> > 
> > We sometimes avoid touching compat/ code for style issues because it's
> > copied from elsewhere. And diverging from upstream is more evil than a
> > pure style issue. So potentially we could drop these hunks (though I
> > think maybe mingw is our own thing?).
> > 
> > >  contrib/coccinelle/noextern.cocci |   6 +
> > 
> > I have mixed feelings on this cocci script.
> 
> I have actual bad experience with this :)
> 
> v4 of this patch series excluded 'compat/' from the conversion, but
> the semantic patch is applied to 'compat/' all the same, resulting in
> failed CI builds because of the four 'extern's in 'compat/obstack.h',
> and will continue to do so.
> 
> (Coccinelle has no issues with those other header files; I guess those
> are not included in the '.c' source files we analyze with Coccinelle
> in a stock Linux build environment).

Since this is the case, we should drop 4/4 because it is not only
unhelpful, because it doesn't scan header files, but actively harmful.
The cocci script used is in the log for 1/4 anyway.

Thanks for checking on this,

Denton

> 
> 
> > I'm happy to _see_ it, as
> > it's important to show how the transformation was done. But for most of
> > the other scripts, we expect programmers to introduce new cases that
> > need converting, and we'd like to catch those automatically. Here I find
> > it reasonably unlikely for a lot of "extern" to slip in, with the
> > exception of some topics in flight.
> > 
> > And these coccinelle scripts are kind of expensive to run. So I wonder
> > if the tradeoff is worth it here (perhaps it is now, as we catch those
> > topics in flight, it might be worth dropping this one in a few months).
> > 
> > At any rate, thanks for doing all of this tedious work. :)
> > 
> > -Peff