On Sun, Oct 16, 2016 at 01:35:34PM +0200, Theo Buehler wrote:
> On Sun, Oct 16, 2016 at 11:47:32AM +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.
> 
> Here's a slightly improved version: I failed to remove a few newlines in
> warning/error strings in main.c.

Looks good to me, but see some nits below.


> 
> 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 11:25:20 -0000
> @@ -75,7 +75,7 @@ static void optiondelta(void);
>  
>  int  verbose;
>  
> -void
> +__dead void
>  usage(void)

The usage declaration in ukc.c needs updating too.

        __dead void usage(void);


>  {
>       extern char *__progname;
> @@ -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:", builddir);

No ":" at the end of format. This will be added by err() automatically.


> +     } else if (!S_ISDIR(st.st_mode))
> +             errx(2, "%s is not a directory", builddir);
> +     if (chdir(builddir) != 0)
> +             errx(2, "cannot change to %s", builddir);
> +     if (stat(srcdir, &st) != 0 || !S_ISDIR(st.st_mode))
> +             errx(2, "%s is not a directory", srcdir);
>  
>       if (bflag) {
>               if (pledge("stdio rpath wpath cpath flock", NULL) == -1)
> 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 11:25:20 -0000
> @@ -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);

return(emitwarn("writ", fname, NULL)); ?


>       }
>       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);
>  }
>  
> @@ -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);

return(emitwarn("writ", fname, NULL)); ?


>       }
>       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);

Reply via email to