On Sat, Apr 27, 2013 at 08:10:41AM +0200, Otto Moerbeek wrote:
> On Sat, Apr 27, 2013 at 01:08:06AM -0400, Eitan Adler wrote:
>
> > Hey all,
> >
> > Time for attempt #2!
> >
> > Adding static to internal function allows the compiler to better
> > detect dead code (functions, variables, etc) and makes it easier for
> > the compiler to optimize; e.g., since it knows a function will only
> > called once it can inline code; or not output a symbol for a certain
> > function.
>
> In general we don't lik this because it makes things harder to debug.
> For libraries, yes, but for programs, no.
>
> -Otto
+1. We see way more 'nuke stupid static crap' diffs that 'add static'
diffs. We are even dubious about almost all inline functions since
they are also harder to debug and (on most 'modern' archs) add
little if any performance but do make executables bigger. Just in
case you have a 'use inline functions to speed things up just like
XBSD' diff in the queue, and were about to hit another sensitive
button issue. :-)
.... Ken
>
> >
> > I'm offering this patch for review:
> >
> > Index: rm.c
> > ===================================================================
> > RCS file: /cvs/src/bin/rm/rm.c,v
> > retrieving revision 1.27
> > diff -u -r1.27 rm.c
> > --- rm.c 5 Sep 2012 19:49:08 -0000 1.27
> > +++ rm.c 27 Apr 2013 04:26:18 -0000
> > @@ -49,15 +49,15 @@
> >
> > extern char *__progname;
> >
> > -int dflag, eval, fflag, iflag, Pflag, stdin_ok;
> > +static int dflag, eval, fflag, iflag, Pflag, stdin_ok;
> >
> > -int check(char *, char *, struct stat *);
> > -void checkdot(char **);
> > -void rm_file(char **);
> > -int rm_overwrite(char *, struct stat *);
> > -int pass(int, off_t, char *, size_t);
> > -void rm_tree(char **);
> > -void usage(void);
> > +static int check(char *, char *, struct stat *);
> > +static void checkdot(char **);
> > +static void rm_file(char **);
> > +static int rm_overwrite(char *, struct stat *);
> > +static int pass(int, off_t, char *, size_t);
> > +static void rm_tree(char **);
> > +static void usage(void);
> >
> > /*
> > * rm --
> > @@ -117,7 +117,7 @@
> > exit (eval);
> > }
> >
> > -void
> > +static void
> > rm_tree(char **argv)
> > {
> > FTS *fts;
> > @@ -217,7 +217,7 @@
> > fts_close(fts);
> > }
> >
> > -void
> > +static void
> > rm_file(char **argv)
> > {
> > struct stat sb;
> > @@ -271,7 +271,7 @@
> > * kernel support.
> > * Returns 1 for success.
> > */
> > -int
> > +static int
> > rm_overwrite(char *file, struct stat *sbp)
> > {
> > struct stat sb, sb2;
> > @@ -324,7 +324,7 @@
> > return (0);
> > }
> >
> > -int
> > +static int
> > pass(int fd, off_t len, char *buf, size_t bsize)
> > {
> > size_t wlen;
> > @@ -338,7 +338,7 @@
> > return (1);
> > }
> >
> > -int
> > +static int
> > check(char *path, char *name, struct stat *sp)
> > {
> > int ch, first;
> > @@ -380,7 +380,7 @@
> > * trailing slashes have been removed, we'll remove them here.
> > */
> > #define ISDOT(a) ((a)[0] == '.' && (!(a)[1] || ((a)[1] == '.' &&
> > !(a)[2])))
> > -void
> > +static void
> > checkdot(char **argv)
> > {
> > char *p, **save, **t;
> > @@ -411,7 +411,7 @@
> > }
> > }
> >
> > -void
> > +static void
> > usage(void)
> > {
> > (void)fprintf(stderr, "usage: %s [-dfiPRr] file ...\n", __progname);
> >
> > --
> > Eitan Adler
>
