Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 14 Dec 2009 03:45:42 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Xin LI <delphij@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r200465 - head/usr.bin/xinstall
Message-ID:  <20091214031938.E33347@delplex.bde.org>
In-Reply-To: <200912130334.nBD3YJbO072556@svn.freebsd.org>
References:  <200912130334.nBD3YJbO072556@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 13 Dec 2009, Xin LI wrote:

> Log:
>  Staticify internal functions and make usage() a prototype.

Please actually declare them static.

> Modified: head/usr.bin/xinstall/xinstall.c
> ==============================================================================
> --- head/usr.bin/xinstall/xinstall.c	Sun Dec 13 03:29:05 2009	(r200464)
> +++ head/usr.bin/xinstall/xinstall.c	Sun Dec 13 03:34:19 2009	(r200465)
> @@ -86,16 +86,16 @@ int dobackup, docompare, dodir, dopreser
> mode_t mode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
> const char *suffix = BACKUP_SUFFIX;
>
> -void	copy(int, const char *, int, const char *, off_t);
> -int	compare(int, const char *, size_t, int, const char *, size_t);
> -int	create_newfile(const char *, int, struct stat *);
> -int	create_tempfile(const char *, char *, size_t);
> -void	install(const char *, const char *, u_long, u_int);
> -void	install_dir(char *);
> -u_long	numeric_id(const char *, const char *);
> -void	strip(const char *);
> -int	trymmap(int);
> -void	usage(void);
> +static void	copy(int, const char *, int, const char *, off_t);
> +static int	compare(int, const char *, size_t, int, const char *, size_t);

Sorting errors like copy < compare could be fixed when making changes to
all the prototypes like this.

> +static int	create_newfile(const char *, int, struct stat *);
> +static int	create_tempfile(const char *, char *, size_t);
> +static void	install(const char *, const char *, u_long, u_int);
> +static void	install_dir(char *);
> +static u_long	numeric_id(const char *, const char *);
> +static void	strip(const char *);
> +static int	trymmap(int);
> +static void	usage(void);
>
> int
> main(int argc, char *argv[])
> @@ -771,7 +771,7 @@ install_dir(char *path)
>  *	print a usage message and die
>  */
> void
> -usage()
> +usage(void)
> {
> 	(void)fprintf(stderr,
> "usage: install [-bCcpSsv] [-B suffix] [-f flags] [-g group] [-m mode]\n"

It is a good obfuscation to declare functions as static only in the
prototype, so that you can't see the static for the actual function.
The reverse obfuscation (with static only in the function definition)
would make more sense, but is a constraint error.  (C99 allows building
up a declaration by accumulating type or linkage details in some cases
but not here.  Last time I checked, gcc didn't understand this, and
warns about "redundant redeclarations" for non-redundant non-re
declarations.)

With C90 function definitions (not prototypes!) and almost all functions
staticized, most of foward declarations involving prototypes are
unnecessary.  We couldn't agree if style requires having them (it
is sometimes useful to have a complete list of static functions as
prototypes near the top of the file).  We should agree before large
staicization sweeps.  The bad style of having everything extern,
combined prototyping all extern functions, left us many thousands of
lines of non-static prototypes near the tops of files.  (In 4.4BSD,
prototypes had to be done like that, using __P(()), even for static
functions, since that was the only reasonable way to ifdef prototypes.
After use of __P(()) was removed, extern prototypes still had to be
done like that since -Wmissing-prototypes forces it at high WARNS
even for functions that should be static and are defined before they
are called.  But static prototypes don't have to be done like that.)

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20091214031938.E33347>