On Sun, 16 Oct 2016 11:47:32 +0200, Theo Buehler wrote:

> Many files already include <err.h> and there's a mix of hand-rolled
> warning messages and there's incorrect usage warn("config: ..."). This
> is a first sweep at unifying them.
> 
> In mkheaders.c, there is an err() function, rename it to emitwarn()
> since there are emit* functions already and it is non-fatal.

Some minor issues noted inline but otherwise OK.

 - todd

> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/config/main.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 main.c
> --- main.c    16 Oct 2016 09:36:46 -0000      1.55
> +++ main.c    16 Oct 2016 09:42:01 -0000
> @@ -75,7 +75,7 @@ static void optiondelta(void);
>  
>  int  verbose;
>  
> -void
> +__dead void
>  usage(void)
>  {
>       extern char *__progname;
> @@ -169,19 +169,15 @@ main(int argc, char *argv[])
>  
>       if (eflag) {
>  #ifdef MAKE_BOOTSTRAP
> -             fprintf(stderr, "config: UKC not available in this binary\n");
> -             exit(1);
> +             errx(1, "UKC not available in this binary");
>  #else
>               return (ukc(argv[0], outfile, uflag, fflag));
>  #endif
>       }
>  
>       conffile = (argc == 1) ? argv[0] : "CONFIG";
> -     if (firstfile(conffile)) {
> -             (void)fprintf(stderr, "config: cannot read %s: %s\n",
> -                 conffile, strerror(errno));
> -             exit(2);
> -     }
> +     if (firstfile(conffile))
> +             err(2, "cannot read %s\n", conffile);

You don't want that trailing newline.

>  
>       /*
>        * Init variables.
> @@ -246,8 +242,7 @@ main(int argc, char *argv[])
>                           defmaxusers);
>                       maxusers = defmaxusers;
>               } else {
> -                     (void)fprintf(stderr,
> -                         "config: need \"maxusers\" line\n");
> +                     warnx("need \"maxusers\" line");
>                       errors++;
>               }
>       }
> @@ -269,7 +264,7 @@ main(int argc, char *argv[])
>           mkioconf())
>               stop();
>       optiondelta();
> -     exit(0);
> +     return (0);
>  }
>  
>  static int
> @@ -278,11 +273,11 @@ mksymlink(const char *value, const char 
>       int ret = 0;
>  
>       if (remove(path) && errno != ENOENT) {
> -             warn("config: remove(%s)", path);
> +             warn("remove(%s)", path);
>               ret = 1;
>       }
>       if (symlink(value, path)) {
> -             warn("config: symlink(%s -> %s)", path, value);
> +             warn("symlink(%s -> %s)", path, value);
>               ret = 1;
>       }
>       return (ret);
> @@ -624,8 +619,7 @@ badstar(void)
>               continue;
>       foundstar:
>               if (ht_lookup(needcnttab, d->d_name)) {
> -                     (void)fprintf(stderr,
> -                 "config: %s's cannot be *'d until its driver is fixed\n",
> +                     warnx("%s's cannot be *'d until its driver is fixed",
>                           d->d_name);
>                       errs++;
>                       continue;
> @@ -662,26 +656,14 @@ setupdirs(void)
>               builddir = defbuilddir;
>  
>       if (stat(builddir, &st) != 0) {
> -             if (mkdir(builddir, 0777)) {
> -                     (void)fprintf(stderr, "config: cannot create %s: %s\n",
> -                         builddir, strerror(errno));
> -                     exit(2);
> -             }
> -     } else if (!S_ISDIR(st.st_mode)) {
> -             (void)fprintf(stderr, "config: %s is not a directory\n",
> -                 builddir);
> -             exit(2);
> -     }
> -     if (chdir(builddir) != 0) {
> -             (void)fprintf(stderr, "config: cannot change to %s\n",
> -                 builddir);
> -             exit(2);
> -     }
> -     if (stat(srcdir, &st) != 0 || !S_ISDIR(st.st_mode)) {
> -             (void)fprintf(stderr, "config: %s is not a directory\n",
> -                 srcdir);
> -             exit(2);
> -     }
> +             if (mkdir(builddir, 0777))
> +                     err(2, "cannot create %s:\n", builddir);

Another trailing newline, also the ':' be removed.

> +     } else if (!S_ISDIR(st.st_mode))
> +             errx(2, "%s is not a directory\n", builddir);

Another trailing newline.  You might also consider:

        errc(2, ENOTDIR, "%s", builddir);

> +     if (chdir(builddir) != 0)
> +             errx(2, "cannot change to %s\n", builddir);

Another trailing newline.

> +     if (stat(srcdir, &st) != 0 || !S_ISDIR(st.st_mode))
> +             errx(2, "%s is not a directory\n", srcdir);

Another trailing newline, maybe use errc().

>  
>       if (bflag) {
>               if (pledge("stdio rpath wpath cpath flock", NULL) == -1)
> @@ -693,14 +675,10 @@ setupdirs(void)
>               goto reconfig;
>  
>       fp = fopen("Makefile", "w");
> -     if (!fp) {
> -             (void)fprintf(stderr, "config: cannot create Makefile\n");
> -             exit(2);
> -     }
> -     if (fprintf(fp, ".include \"../Makefile.inc\"\n") < 0) {
> -             (void)fprintf(stderr, "config: cannot create Makefile\n");
> -             exit(2);
> -     }
> +     if (!fp)
> +             errx(2, "cannot create Makefile");
> +     if (fprintf(fp, ".include \"../Makefile.inc\"\n") < 0)
> +             errx(2, "cannot create Makefile");
>       fclose(fp);

That check for fprintf() failure is almost guaranteed never to
trigger.  The actual write will be done in fclose().  Perhaps:

if (fprintf(fp, ".include \"../Makefile.inc\"\n") < 0 || fclose(fp) == EOF)
        errx(2, "cannot write Makefile");

Not a new problem so you can ignore this for now if you want.  Other
options include fflush() + check ferror() before fclose().

>  
>  reconfig:
> Index: mkheaders.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/config/mkheaders.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 mkheaders.c
> --- mkheaders.c       16 Jan 2015 06:40:16 -0000      1.21
> +++ mkheaders.c       16 Oct 2016 09:42:01 -0000
> @@ -42,6 +42,7 @@
>   */
>  
>  #include <ctype.h>
> +#include <err.h>
>  #include <errno.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -51,7 +52,7 @@
>  
>  static int emitcnt(struct nvlist *);
>  static int emitopt(struct nvlist *);
> -static int err(const char *, char *, FILE *);
> +static int emitwarn(const char *, char *, FILE *);
>  static char *cntname(const char *);
>  
>  /*
> @@ -102,22 +103,21 @@ emitcnt(struct nvlist *head)
>               nv = nv->nv_next;
>       }
>       if (ferror(fp))
> -             return (err("read", fname, fp));
> +             return (emitwarn("read", fname, fp));
>       (void)fclose(fp);
>       if (nv == NULL)
>               return (0);
>  writeit:
>       if ((fp = fopen(fname, "w")) == NULL) {
> -             (void)fprintf(stderr, "config: cannot write %s: %s\n",
> -                 fname, strerror(errno));
> +             warn("cannot write %s", fname);
>               return (1);
>       }
>       for (nv = head; nv != NULL; nv = nv->nv_next)
>               if (fprintf(fp, "#define\t%s\t%d\n",
>                   cntname(nv->nv_name), nv->nv_int) < 0)
> -                     return (err("writ", fname, fp));
> +                     return (emitwarn("writ", fname, fp));
>       if (fclose(fp))
> -             return (err("writ", fname, NULL));
> +             return (emitwarn("writ", fname, NULL));
>       return (0);
>  }
>  
> @@ -149,7 +149,7 @@ emitopt(struct nvlist *nv)
>       }
>  
>       if (totlen < 0 || totlen >= sizeof new_contents) {
> -             fprintf(stderr, "config: string too long\n");
> +             warnx("string too long");
>               return (1);
>       }
>  
> @@ -166,7 +166,7 @@ emitopt(struct nvlist *nv)
>                       goto writeit;
>       }
>       if (ferror(fp))
> -             return (err("read", fname, fp));
> +             return (emitwarn("read", fname, fp));
>       (void)fclose(fp);
>       if (nlines == 1)
>               return (0);
> @@ -175,23 +175,21 @@ writeit:
>        * They're different, or the file doesn't exist.
>        */
>       if ((fp = fopen(fname, "w")) == NULL) {
> -             (void)fprintf(stderr, "config: cannot write %s: %s\n",
> -                 fname, strerror(errno));
> +             warn("cannot write %s", fname);
>               return (1);
>       }
>       if (fprintf(fp, "%s", new_contents) < 0)
> -             return (err("writ", fname, fp));
> +             return (emitwarn("writ", fname, fp));
>       if (fclose(fp))
> -             return (err("writ", fname, fp));
> +             return (emitwarn("writ", fname, fp));
>       return (0);
>  }
>  
>  static int
> -err(const char *what, char *fname, FILE *fp)
> +emitwarn(const char *what, char *fname, FILE *fp)
>  {
>  
> -     (void)fprintf(stderr, "config: error %sing %s: %s\n",
> -         what, fname, strerror(errno));
> +     warn("error %sing %s", what, fname);
>       if (fp)
>               (void)fclose(fp);
>       return (1);
> Index: mkioconf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/config/mkioconf.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 mkioconf.c
> --- mkioconf.c        11 Sep 2015 07:13:58 -0000      1.34
> +++ mkioconf.c        16 Oct 2016 09:42:01 -0000
> @@ -41,6 +41,7 @@
>   *   from: @(#)mkioconf.c    8.1 (Berkeley) 6/6/93
>   */
>  
> +#include <err.h>
>  #include <errno.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -77,8 +78,7 @@ mkioconf(void)
>  
>       qsort(packed, npacked, sizeof *packed, cforder);
>       if ((fp = fopen("ioconf.c", "w")) == NULL) {
> -             (void)fprintf(stderr, "config: cannot write ioconf.c: %s\n",
> -                 strerror(errno));
> +             warn("cannot write ioconf.c");
>               return (1);
>       }
>       v = emithdr(fp);
> @@ -86,9 +86,7 @@ mkioconf(void)
>           emitlocnames(fp) || emitpv(fp) || emitcfdata(fp) ||
>           emitroots(fp) || emitpseudo(fp)) {
>               if (v >= 0)
> -                     (void)fprintf(stderr,
> -                         "config: error writing ioconf.c: %s\n",
> -                         strerror(errno));
> +                     warn("error writing ioconf.c");
>               (void)fclose(fp);
>               /* (void)unlink("ioconf.c"); */
>               return (1);
> @@ -127,8 +125,7 @@ emithdr(FILE *ofp)
>                       if (fwrite(buf, 1, n, ofp) != n)
>                               return (1);
>               if (ferror(ifp)) {
> -                     (void)fprintf(stderr, "config: error reading %s: %s\n",
> -                         ifn, strerror(errno));
> +                     warn("error reading %s", ifn);
>                       (void)fclose(ifp);
>                       return (-1);
>               }
> @@ -425,9 +422,7 @@ emitroots(FILE *fp)
>                       continue;
>               if (i->i_unit != 0 &&
>                   (i->i_unit != STAR || i->i_base->d_umax != 0))
> -                     (void)fprintf(stderr,
> -                         "config: warning: `%s at root' is not unit 0\n",
> -                         i->i_name);
> +                     warnx("warning: `%s at root' is not unit 0", i->i_name)
> ;
>               if (fprintf(fp, "\t%2d /* %s */,\n",
>                   i->i_cfindex, i->i_name) < 0)
>                       return (1);
> Index: mkmakefile.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/config/mkmakefile.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 mkmakefile.c
> --- mkmakefile.c      16 Jan 2015 06:40:16 -0000      1.41
> +++ mkmakefile.c      16 Oct 2016 09:42:01 -0000
> @@ -80,14 +80,12 @@ mkmakefile(void)
>           machine, machine);
>       ifname = sourcepath(buf);
>       if ((ifp = fopen(ifname, "r")) == NULL) {
> -             (void)fprintf(stderr, "config: cannot read %s: %s\n",
> -                 ifname, strerror(errno));
> +             warn("cannot read %s", ifname);
>               free(ifname);
>               return (1);
>       }
>       if ((ofp = fopen("Makefile", "w")) == NULL) {
> -             (void)fprintf(stderr, "config: cannot write Makefile: %s\n",
> -                 strerror(errno));
> +             warn("cannot write Makefile");
>               free(ifname);
>               (void)fclose(ifp);
>               return (1);
> @@ -125,9 +123,7 @@ mkmakefile(void)
>                       goto wrerror;
>       }
>       if (ferror(ifp)) {
> -             (void)fprintf(stderr,
> -                 "config: error reading %s (at line %d): %s\n",
> -                 ifname, lineno, strerror(errno));
> +             warn("error reading %s (at line %d)", ifname, lineno);
>               goto bad;
>       }
>       if (fclose(ofp)) {
> @@ -138,8 +134,7 @@ mkmakefile(void)
>       free(ifname);
>       return (0);
>  wrerror:
> -     (void)fprintf(stderr, "config: error writing Makefile: %s\n",
> -         strerror(errno));
> +     warn("error writing Makefile");
>  bad:
>       if (ofp != NULL)
>               (void)fclose(ofp);
> Index: mkswap.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/config/mkswap.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 mkswap.c
> --- mkswap.c  16 Jan 2015 06:40:16 -0000      1.15
> +++ mkswap.c  16 Oct 2016 09:42:01 -0000
> @@ -43,6 +43,7 @@
>  
>  #include <sys/param.h>       /* NODEV */
>  
> +#include <err.h>
>  #include <errno.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -89,8 +90,7 @@ mkoneswap(struct config *cf)
>  
>       (void)snprintf(fname, sizeof fname, "swap%s.c", cf->cf_name);
>       if ((fp = fopen(fname, "w")) == NULL) {
> -             (void)fprintf(stderr, "config: cannot write %s: %s\n",
> -                 fname, strerror(errno));
> +             warn("cannot write %s", fname);
>               return (1);
>       }
>       if (fputs("\
> @@ -125,8 +125,7 @@ mkoneswap(struct config *cf)
>       }
>       return (0);
>  wrerror:
> -     (void)fprintf(stderr, "config: error writing %s: %s\n",
> -         fname, strerror(errno));
> +     warn("error writing %s", fname);
>       if (fp != NULL)
>               (void)fclose(fp);
>       /* (void)unlink(fname); */
> Index: ukc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/config/ukc.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 ukc.c
> --- ukc.c     12 Oct 2015 04:43:30 -0000      1.20
> +++ ukc.c     16 Oct 2016 09:42:01 -0000
> @@ -57,7 +57,6 @@ check_int(int idx, const char *name)
>  int
>  ukc(char *file, char *outfile, int uflag, int force)
>  {
> -     extern char *__progname;
>       int i;
>       kvm_t *kd;
>       char errbuf[_POSIX2_LINE_MAX];
> @@ -65,7 +64,7 @@ ukc(char *file, char *outfile, int uflag
>       char history[1024], kversion[1024];
>  
>       if (file == NULL) {
> -             fprintf(stderr, "%s: no file specified\n", __progname);
> +             warnx("no file specified");
>               usage();
>       }
>  
> 

Reply via email to