[Diffutils-devel] FYI: [PATCH] maint: use die rather than error

[Jim Meyering]

FYI, I've just pushed this:

>From 1c1de418606f163a0027b3e7305ca91708616219 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 16 Oct 2016 08:43:14 -0700
Subject: [PATCH] maint: use die rather than error

Use "die (N, ..." rather than "error (N, ..." whenever N is a nonzero
constant.  That lets the compiler know that control never goes beyond
that point, and thus makes unnecessary the occasional following
"abort ();" or "break;" statement we have historically added to inform
static analysis tools of this aspect of "error" semantics.
* src/die.h: New file.
* src/Makefile.am (noinst_HEADERS): Add it.
* src/cmp.c: Use die in place of error whenever the first
argument is a nonzero constant.  Also remove any immediately-
following call to abort, and include "die.h".
* src/diff.c: Likewise.
* src/diff3.c: Likewise.
* src/sdiff.c: Likewise.
* src/util.c: Likewise.
---
 src/Makefile.am |  5 ++++-
 src/cmp.c       | 18 +++++++++---------
 src/die.h       | 31 +++++++++++++++++++++++++++++++
 src/diff.c      |  6 +++---
 src/diff3.c     | 16 +++++++---------
 src/sdiff.c     |  4 ++--
 src/util.c      |  9 ++++-----
 7 files changed, 60 insertions(+), 29 deletions(-)
 create mode 100644 src/die.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 167f7fd..65a1633 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -43,7 +43,10 @@ sdiff_SOURCES = sdiff.c
 diff_SOURCES = \
   analyze.c context.c diff.c dir.c ed.c ifdef.c io.c \
   normal.c side.c util.c
-noinst_HEADERS = diff.h system.h
+noinst_HEADERS =       \
+  die.h                        \
+  diff.h               \
+  system.h

 MOSTLYCLEANFILES = paths.h paths.ht

diff --git a/src/cmp.c b/src/cmp.c
index ba5553b..5c1493d 100644
--- a/src/cmp.c
+++ b/src/cmp.c
@@ -23,6 +23,7 @@

 #include <c-stack.h>
 #include <cmpbuf.h>
+#include "die.h"
 #include <error.h>
 #include <exitfail.h>
 #include <file-type.h>
@@ -114,9 +115,8 @@ try_help (char const *reason_msgid, char const *operand)
 {
   if (reason_msgid)
     error (0, 0, _(reason_msgid), operand);
-  error (EXIT_TROUBLE, 0,
+  die (EXIT_TROUBLE, 0,
         _("Try '%s --help' for more information."), program_name);
-  abort ();
 }

 static char const valid_suffixes[] = "kKMGTPEZY0";
@@ -152,9 +152,9 @@ static void
 check_stdout (void)
 {
   if (ferror (stdout))
-    error (EXIT_TROUBLE, 0, "%s", _("write failed"));
+    die (EXIT_TROUBLE, 0, "%s", _("write failed"));
   else if (fclose (stdout) != 0)
-    error (EXIT_TROUBLE, errno, "%s", _("standard output"));
+    die (EXIT_TROUBLE, errno, "%s", _("standard output"));
 }

 static char const * const option_help_msgid[] = {
@@ -303,7 +303,7 @@ main (int argc, char **argv)
          if (file_desc[f1] < 0 && comparison_type == type_status)
            exit (EXIT_TROUBLE);
          else
-           error (EXIT_TROUBLE, errno, "%s", file[f1]);
+           die (EXIT_TROUBLE, errno, "%s", file[f1]);
        }
     }

@@ -363,7 +363,7 @@ main (int argc, char **argv)

   for (f = 0; f < 2; f++)
     if (close (file_desc[f]) != 0)
-      error (EXIT_TROUBLE, errno, "%s", file[f]);
+      die (EXIT_TROUBLE, errno, "%s", file[f]);
   if (exit_status != EXIT_SUCCESS && comparison_type < type_no_stdout)
     check_stdout ();
   exit (exit_status);
@@ -421,7 +421,7 @@ cmp (void)
              if (r != bytes_to_read)
                {
                  if (r == SIZE_MAX)
-                   error (EXIT_TROUBLE, errno, "%s", file[f]);
+                   die (EXIT_TROUBLE, errno, "%s", file[f]);
                  break;
                }
              ig -= r;
@@ -443,10 +443,10 @@ cmp (void)

       read0 = block_read (file_desc[0], buf0, bytes_to_read);
       if (read0 == SIZE_MAX)
-       error (EXIT_TROUBLE, errno, "%s", file[0]);
+       die (EXIT_TROUBLE, errno, "%s", file[0]);
       read1 = block_read (file_desc[1], buf1, bytes_to_read);
       if (read1 == SIZE_MAX)
-       error (EXIT_TROUBLE, errno, "%s", file[1]);
+       die (EXIT_TROUBLE, errno, "%s", file[1]);

       smaller = MIN (read0, read1);

diff --git a/src/die.h b/src/die.h
new file mode 100644
index 0000000..f64ab34
--- /dev/null
+++ b/src/die.h
@@ -0,0 +1,31 @@
+/* Report an error and exit.
+   Copyright 2016 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA
+   02110-1301, USA.  */
+
+#ifndef DIE_H
+# define DIE_H
+
+# include <error.h>
+# include <stdbool.h>
+# include <verify.h>
+
+/* Like 'error (STATUS, ...)', except STATUS must be a nonzero constant.
+   This may pacify the compiler or help it generate better code.  */
+# define die(status, ...) \
+  verify_expr (status, (error (status, __VA_ARGS__), assume (false)))
+
+#endif /* DIE_H */
diff --git a/src/diff.c b/src/diff.c
index 686945e..bc7fd9e 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -20,6 +20,7 @@

 #define GDIFF_MAIN
 #include "diff.h"
+#include "die.h"
 #include <assert.h>
 #include "paths.h"
 #include <c-stack.h>
@@ -862,7 +863,7 @@ summarize_regexp_list (struct regexp_list *reglist)
          char const *m = re_compile_pattern (reglist->regexps, reglist->len,
                                              reglist->buf);
          if (m)
-           error (EXIT_TROUBLE, 0, "%s: %s", reglist->regexps, m);
+           die (EXIT_TROUBLE, 0, "%s: %s", reglist->regexps, m);
        }
     }
 }
@@ -872,9 +873,8 @@ try_help (char const *reason_msgid, char const *operand)
 {
   if (reason_msgid)
     error (0, 0, _(reason_msgid), operand);
-  error (EXIT_TROUBLE, 0, _("Try '%s --help' for more information."),
+  die (EXIT_TROUBLE, 0, _("Try '%s --help' for more information."),
         program_name);
-  abort ();
 }

 static void
diff --git a/src/diff3.c b/src/diff3.c
index 055d275..7f93d22 100644
--- a/src/diff3.c
+++ b/src/diff3.c
@@ -24,6 +24,7 @@

 #include <c-stack.h>
 #include <cmpbuf.h>
+#include "die.h"
 #include <error.h>
 #include <exitfail.h>
 #include <file-type.h>
@@ -403,7 +404,7 @@ main (int argc, char **argv)
        if (stat (file[i], &statb) < 0)
          perror_with_exit (file[i]);
        else if (S_ISDIR (statb.st_mode))
-         error (EXIT_TROUBLE, EISDIR, "%s", file[i]);
+         die (EXIT_TROUBLE, EISDIR, "%s", file[i]);
       }

 #ifdef SIGCHLD
@@ -457,9 +458,8 @@ try_help (char const *reason_msgid, char const *operand)
 {
   if (reason_msgid)
     error (0, 0, _(reason_msgid), operand);
-  error (EXIT_TROUBLE, 0,
+  die (EXIT_TROUBLE, 0,
         _("Try '%s --help' for more information."), program_name);
-  abort ();
 }

 static void
@@ -1319,7 +1319,7 @@ read_diff (char const *filea,
   status = ! werrno && WIFEXITED (wstatus) ? WEXITSTATUS (wstatus) : INT_MAX;

   if (EXIT_TROUBLE <= status)
-    error (EXIT_TROUBLE, werrno,
+    die (EXIT_TROUBLE, werrno,
           _(status == 126
             ? "subsidiary program '%s' could not be invoked"
             : status == 127
@@ -1776,17 +1776,15 @@ reverse_diff3_blocklist (struct diff3_block *diff)

   return prev;
 }
-
+
 static void
 fatal (char const *msgid)
 {
-  error (EXIT_TROUBLE, 0, "%s", _(msgid));
-  abort ();
+  die (EXIT_TROUBLE, 0, "%s", _(msgid));
 }

 static void
 perror_with_exit (char const *string)
 {
-  error (EXIT_TROUBLE, errno, "%s", string);
-  abort ();
+  die (EXIT_TROUBLE, errno, "%s", string);
 }
diff --git a/src/sdiff.c b/src/sdiff.c
index d900779..aa69f18 100644
--- a/src/sdiff.c
+++ b/src/sdiff.c
@@ -26,6 +26,7 @@

 #include <c-stack.h>
 #include <dirname.h>
+#include "die.h"
 #include <error.h>
 #include <exitfail.h>
 #include <file-type.h>
@@ -155,9 +156,8 @@ try_help (char const *reason_msgid, char const *operand)
 {
   if (reason_msgid)
     error (0, 0, _(reason_msgid), operand);
-  error (EXIT_TROUBLE, 0, _("Try '%s --help' for more information."),
+  die (EXIT_TROUBLE, 0, _("Try '%s --help' for more information."),
         program_name);
-  abort ();
 }

 static void
diff --git a/src/util.c b/src/util.c
index c95dc77..ca6fbfa 100644
--- a/src/util.c
+++ b/src/util.c
@@ -20,6 +20,7 @@

 #include "diff.h"
 #include "argmatch.h"
+#include "die.h"
 #include <dirname.h>
 #include <error.h>
 #include <system-quote.h>
@@ -77,8 +78,7 @@ pfatal_with_name (char const *name)
 {
   int e = errno;
   print_message_queue ();
-  error (EXIT_TROUBLE, e, "%s", name);
-  abort ();
+  die (EXIT_TROUBLE, e, "%s", name);
 }

 /* Print an error message containing MSGID, then exit.  */
@@ -87,8 +87,7 @@ void
 fatal (char const *msgid)
 {
   print_message_queue ();
-  error (EXIT_TROUBLE, 0, "%s", _(msgid));
-  abort ();
+  die (EXIT_TROUBLE, 0, "%s", _(msgid));
 }
 
 /* Like printf, except if -l in effect then save the message and print later.
@@ -965,7 +964,7 @@ finish_output (void)
                ? WEXITSTATUS (wstatus)
                : INT_MAX);
       if (status)
-       error (EXIT_TROUBLE, werrno,
+       die (EXIT_TROUBLE, werrno,
               _(status == 126
                 ? "subsidiary program '%s' could not be invoked"
                 : status == 127
-- 
2.7.4