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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: May 3, 2019, 14:42 UTC
Message-ID: <20190503144211.GH14763@szeder.dev>
In-Reply-To: <nycvar.QRO.7.76.6.1905031127170.45@tvgsbejvaqbjf.bet>

On Fri, May 03, 2019 at 11:32:32AM +0200, Johannes Schindelin wrote:

Show 21 quoted lines

> Hi,
> 
> On Thu, 2 May 2019, SZEDER Gábor wrote:
> 
> > On Wed, May 01, 2019 at 06:01:08AM -0400, Denton Liu wrote:
> > > > Is it not possible to exclude certain directories for certain semantic
> > > > patches?
> > > >
> > > > I guess we could also simply declare that *all* Coccinelle patches should
> > > > leave `compat/` alone, on the basis that those files are likely coming
> > > > from some sort of upstream. But then, `compat/mingw.c` and `compat/win32/`
> > > > seem not to fall into that category...
> > > >
> > > > Ciao,
> > > > Dscho
> > >
> > > Deciding whether this is a good idea is above my paygrade ;)
> 
> :-)
> 
> As a software developer, you surely have an opinion, though :-D

I don't even have an opinion yet. :)

Show 7 quoted lines

> > FWIW, out of curiosity I've run 'make coccicheck' on Linux with
> > 'compat/mingw.c' and its friends explicitly added to C_SOURCES, and it
> > seems to work...  it even found two places in 'mingw.c' where
> > COPY_ARRAY could replace memcpy() :)
> 
> TBH I had not even known that those files were excluded from coccicheck by
> default.

Well, there was this line in the patch context:

    C_SOURCES = $(patsubst %.o,%.c,$(C_OBJ))

Show 6 quoted lines

> I had assumed that all of Git's sources (and not just the
> Linux-specific ones) were included in the target.
> 
> Since you *could* include it, I now assume that Coccinelle does not need
> to follow the `#include`s (otherwise, it would have complained about not
> finding the `windows.h` header in your setup).

We invoke Coccinelle/spatch with the '--all-includes' option, see the SPATCH_FLAGS make variable. And it does indeed include header files: I've seen it find something to transform in 'cache.h', and then the resulting 'contrib/coccinelle/<whatever>.cocci.patch' included the exact same transformation as many times as the number of files including 'cache.h'... which is a lot :)

I don't really know what can cause 'spatch' to error out (besides unknown command line option or non-existing file specified on the command line), and this is all that 'man spatch' has to say:

       --all-includes
              causes all available include files to be used

Since it explicitly mentions _available_ include files, I would venture to guess that non-available include files are not used, and since it doesn't explicitly mention that such a file causes an error, it doesn't.

Show 6 quoted lines

> If this new assumption is true, I wonder why we cannot make my former
> assumption true as well: why not include *all* of Git's `.c` files in
> `coccicheck`?
> 
> Ciao,
> Dscho