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>