Re: [Outreachy kernel] [PATCH 1/2] Staging: lustre: ptlrpc: Make functions static

[Arnd Bergmann <arnd@arndb.de>]

On Thursday 01 October 2015 15:49:21 Julia Lawall wrote:
> On Thu, 1 Oct 2015, Arnd Bergmann wrote:
> > On Thursday 01 October 2015 15:20:39 Julia Lawall wrote:
> > > On Thu, 1 Oct 2015, Shraddha Barke wrote:
> > > > > Note that there are hundreds more symbols in lustre that should be made
> > > > > static.
> > > > > Most of them are harder to find because they have a declaration in a header
> > > > > file
> > > > > despite being only used in one place.
> > >
> > > Why does it make it hard to find?  One can make a spearate check in .c
> > > files.
> >
> > We currently get a sparse warning for each global identifier definition
> > that is not preceded by a declaration, but since most of our tools
> > (gcc, sparse, coccinelle, ...) work on a single file at a time, they
> > do not know whether a global symbol that is defined in a C file and
> > (incorrectly) declared in a header is used in another C file or not.
> 
> OK, but from Coccinlle you can just call grep, and check the extensions on
> the returned file names, so it is not a big problem.

I'm not following here. How exactly would you find symbols like this with coccinelle?

subsystem.h:

extern int subsystem_state;

subsystem.c:

#include "subsystem.h"
long subsystem_state;

static void function(void)
{
	...
	subsystem_state++;
	...
}

with no users of this variable outside of this file.

> > >  At least not if the header file is shared
> > > between multipe files.  But it still may need to be declared somewhere, if
> > > it is referenced before it is declared in the one file where it is used.
> >
> > I would also consider that a bug we should warn about: basically any
> > 'extern' variable declaration and any forward declaration for a
> > global function (with or without the redundant extern specifier) should
> > be considered a bug. We want the same declaration to be visible by both
> > the file that defines it and the file that uses it so we get warned about
> > type mismatches.
> 
> I think there would be warning about type mismatches anyway, if the
> prototype doesn't match with the definition?

The point is that this only works if all files include the same header.

e.g.

subsystem.h:

extern int subsystem_state;

subsystem.c:

#include "subsystem.h"
long subsystem_state;

driver.c:
/* no #include subsystem.h */
extern long subsystem_state;

	Arnd