Re: [PATCH] swapon: Make needlessly global variables static

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On 2011-01-23 at 20:46:31 +0100, Karel Zak <kzak@xxxxxxxxxx> wrote:
> On Fri, Jan 21, 2011 at 04:44:18PM +0100, Tobias Klauser wrote:
> > Also constify the option array.
> 
>  Please, could be more verbose? Why we need this patch?

Sorry about that.

I was running sparse against util-linux-ng and it warned about the
global symbols:

swapon.c:61:5: warning: symbol 'all' was not declared. Should it be static?
swapon.c:62:5: warning: symbol 'priority' was not declared. Should it be static?
swapon.c:63:5: warning: symbol 'discard' was not declared. Should it be static?
swapon.c:66:5: warning: symbol 'ifexists' was not declared. Should it be static?
swapon.c:67:5: warning: symbol 'fixpgsz' was not declared. Should it be static?
swapon.c:69:5: warning: symbol 'verbose' was not declared. Should it be static?
swapon.c:70:6: warning: symbol 'progname' was not declared. Should it be static?

While there I noticed the option array not being const which usually is
the case as these arrays tend not to be modified.

There are sparse warnings in other parts too. Are you OK with such
patches (e.g. shall I submit them) or is this just nitpicking?

Thanks

> 
>     Karel
> 
> > Signed-off-by: Tobias Klauser <tklauser@xxxxxxxxxx>
> > ---
> >  mount/swapon.c |   18 +++++++++---------
> >  1 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/mount/swapon.c b/mount/swapon.c
> > index e9ccc94..2c0dd9f 100644
> > --- a/mount/swapon.c
> > +++ b/mount/swapon.c
> > @@ -58,18 +58,18 @@ enum {
> >  #define SWAP_SIGNATURE		"SWAPSPACE2"
> >  #define SWAP_SIGNATURE_SZ	(sizeof(SWAP_SIGNATURE) - 1)
> >  
> > -int all;
> > -int priority = -1;	/* non-prioritized swap by default */
> > -int discard;
> > +static int all;
> > +static int priority = -1;	/* non-prioritized swap by default */
> > +static int discard;
> >  
> >  /* If true, don't complain if the device/file doesn't exist */
> > -int ifexists;
> > -int fixpgsz;
> > +static int ifexists;
> > +static int fixpgsz;
> >  
> > -int verbose;
> > -char *progname;
> > +static int verbose;
> > +static char *progname;
> >  
> > -static struct option longswaponopts[] = {
> > +static const struct option longswaponopts[] = {
> >  		/* swapon only */
> >  	{ "priority", required_argument, 0, 'p' },
> >  	{ "discard", 0, 0, 'd' },
> > @@ -84,7 +84,7 @@ static struct option longswaponopts[] = {
> >  	{ NULL, 0, 0, 0 }
> >  };
> >  
> > -static struct option *longswapoffopts = &longswaponopts[4];
> > +static const struct option *longswapoffopts = &longswaponopts[4];
> >  
> >  static int cannot_find(const char *special);
> >  
> > -- 
> > 1.7.0.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe util-linux" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> -- 
>  Karel Zak  <kzak@xxxxxxxxxx>
>  http://karelzak.blogspot.com
> 
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux