diffutils-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 1/5] diff: use variable arg list for messages


From: Paul Eggert
Subject: [PATCH 1/5] diff: use variable arg list for messages
Date: Mon, 30 Aug 2021 10:33:15 -0700

This simplifies the code by using varargs.
* bootstrap.conf (gnulib_modules): Add flexmember.
(XGETTEXT_OPTIONS): Do not flag message5.
* src/util.c: Include flexmember.h, stdarg.h.
(struct msg): New members msgid, argbytes.  args is now
FLEXIBLE_ARRAY_MEMBER, and does not contain msgid.
All uses changed.
(message): Now varargs.  Detect number of args by counting '%'s.
Use FLEXSIZEOF, to avoid problems on systems with buggy
allocators.  Avoid redundant ‘*p = 0’ when *p is already zero
after stpcpy.
(message5): Remove; all callers changed to use ‘message’.
(print_message_queue): Abort if too many args were passed;
this cannot happen with current diffutils.
---
 bootstrap.conf |   2 +-
 src/diff.c     |  20 +++++-----
 src/diff.h     |   4 +-
 src/util.c     | 104 +++++++++++++++++++++++++++++--------------------
 4 files changed, 74 insertions(+), 56 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index e51b2d8..9234a43 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -36,6 +36,7 @@ error
 exclude
 exitfail
 extensions
+flexmember
 fcntl
 fdl
 file-type
@@ -103,7 +104,6 @@ XGETTEXT_OPTIONS=$XGETTEXT_OPTIONS'\\\
  --flag=asprintf:2:c-format\\\
  --from-code=UTF-8\\\
  --flag=message:1:c-format\\\
- --flag=message5:1:c-format\\\
  --flag=try_help:1:c-format\\\
  --flag=vasprintf:2:c-format\\\
  --flag=vasnprintf:3:c-format\\\
diff --git a/src/diff.c b/src/diff.c
index 3b901aa..ef9710a 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -1382,11 +1382,11 @@ compare_files (struct comparison const *parent,
           /* We have two files that are not to be compared.  */
 
           /* See POSIX 1003.1-2001 for this format.  */
-          message5 ("File %s is a %s while file %s is a %s\n",
-                    file_label[0] ? file_label[0] : cmp.file[0].name,
-                    file_type (&cmp.file[0].stat),
-                    file_label[1] ? file_label[1] : cmp.file[1].name,
-                    file_type (&cmp.file[1].stat));
+          message ("File %s is a %s while file %s is a %s\n",
+                  file_label[0] ? file_label[0] : cmp.file[0].name,
+                  file_type (&cmp.file[0].stat),
+                  file_label[1] ? file_label[1] : cmp.file[1].name,
+                  file_type (&cmp.file[1].stat));
 
           /* This is a difference.  */
           status = EXIT_FAILURE;
@@ -1432,11 +1432,11 @@ compare_files (struct comparison const *parent,
           /* We have two files that are not to be compared, because
              one of them is a symbolic link and the other one is not.  */
 
-          message5 ("File %s is a %s while file %s is a %s\n",
-                    file_label[0] ? file_label[0] : cmp.file[0].name,
-                    file_type (&cmp.file[0].stat),
-                    file_label[1] ? file_label[1] : cmp.file[1].name,
-                    file_type (&cmp.file[1].stat));
+          message ("File %s is a %s while file %s is a %s\n",
+                  file_label[0] ? file_label[0] : cmp.file[0].name,
+                  file_type (&cmp.file[0].stat),
+                  file_label[1] ? file_label[1] : cmp.file[1].name,
+                  file_type (&cmp.file[1].stat));
 
           /* This is a difference.  */
           status = EXIT_FAILURE;
diff --git a/src/diff.h b/src/diff.h
index f346b43..0228275 100644
--- a/src/diff.h
+++ b/src/diff.h
@@ -392,9 +392,7 @@ extern void cleanup_signal_handlers (void);
 extern void debug_script (struct change *);
 extern void fatal (char const *) __attribute__((noreturn));
 extern void finish_output (void);
-extern void message (char const *, char const *, char const *);
-extern void message5 (char const *, char const *, char const *,
-                      char const *, char const *);
+extern void message (char const *, ...) _GL_ATTRIBUTE_FORMAT ((printf, 1, 2));
 extern void output_1_line (char const *, char const *, char const *,
                            char const *);
 extern void perror_with_name (char const *);
diff --git a/src/util.c b/src/util.c
index f8b911a..1cb98d4 100644
--- a/src/util.c
+++ b/src/util.c
@@ -19,12 +19,16 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "diff.h"
-#include "argmatch.h"
-#include "die.h"
+
+#include <argmatch.h>
+#include <die.h>
 #include <dirname.h>
 #include <error.h>
+#include <flexmember.h>
 #include <system-quote.h>
 #include <xalloc.h>
+
+#include <stdarg.h>
 #include <signal.h>
 
 /* Use SA_NOCLDSTOP as a proxy for whether the sigaction machinery is
@@ -49,7 +53,15 @@ char const pr_program[] = PR_PROGRAM;
 struct msg
 {
   struct msg *next;
-  char args[1]; /* Format + 4 args, each '\0' terminated, concatenated.  */
+
+  /* Msgid of printf-style format.  */
+  char const *msgid;
+
+  /* Number of bytes in ARGS.  */
+  size_t argbytes;
+
+  /* Arg strings, each '\0' terminated, concatenated.  */
+  char args[FLEXIBLE_ARRAY_MEMBER];
 };
 
 /* Head of the chain of queues messages.  */
@@ -89,42 +101,43 @@ fatal (char const *msgid)
 }
 
 /* Like printf, except if -l in effect then save the message and print later.
+   Also, all arguments must be char * or char const *.
    This is used for things like "Only in ...".  */
 
 void
-message (char const *format_msgid, char const *arg1, char const *arg2)
+message (char const *format_msgid, ...)
 {
-  message5 (format_msgid, arg1, arg2, 0, 0);
-}
+  va_list ap;
+  va_start (ap, format_msgid);
 
-void
-message5 (char const *format_msgid, char const *arg1, char const *arg2,
-          char const *arg3, char const *arg4)
-{
   if (paginate)
     {
-      char *p;
-      char const *arg[5];
-      size_t total_size = offsetof (struct msg, args);
-      struct msg *new;
-
-      arg[0] = format_msgid;
-      arg[1] = arg1;
-      arg[2] = arg2;
-      arg[3] = arg3 ? arg3 : "";
-      arg[4] = arg4 ? arg4 : "";
-
-      for (int i = 0; i < 5; i++)
-        total_size += strlen (arg[i]) + 1;
-
-      new = xmalloc (total_size);
-
-      p = new->args;
-      for (int i = 0; i < 5; i++)
-       {
-         p = stpcpy (p, arg[i]);
-         *p++ = 0;
-       }
+      size_t argbytes = 0;
+
+      for (char const *m = format_msgid; *m; m++)
+       if (*m == '%')
+         {
+           if (m[1] == '%')
+             m++;
+           else
+             argbytes += strlen (va_arg (ap, char const *)) + 1;
+         }
+      va_end (ap);
+
+      struct msg *new = xmalloc (FLEXSIZEOF (struct msg, args, argbytes));
+      new->msgid = format_msgid;
+      new->argbytes = argbytes;
+
+      va_start (ap, format_msgid);
+      char *p = new->args;
+      for (char const *m = format_msgid; *m; m++)
+       if (*m == '%')
+         {
+           if (m[1] == '%')
+             m++;
+           else
+             p = stpcpy (p, va_arg (ap, char const *)) + 1;
+         }
 
       *msg_chain_end = new;
       new->next = 0;
@@ -134,8 +147,10 @@ message5 (char const *format_msgid, char const *arg1, char 
const *arg2,
     {
       if (sdiff_merge_assist)
         putchar (' ');
-      printf (_(format_msgid), arg1, arg2, arg3, arg4);
+      vprintf (_(format_msgid), ap);
     }
+
+  va_end (ap);
 }
 
 /* Output all the messages that were saved up by calls to 'message'.  */
@@ -143,17 +158,22 @@ message5 (char const *format_msgid, char const *arg1, 
char const *arg2,
 void
 print_message_queue (void)
 {
-  char const *arg[5];
-  int i;
-  struct msg *m = msg_chain;
-
-  while (m)
+  for (struct msg *m = msg_chain; m; )
     {
+      /* Change this if diff ever has messages with more than 4 args.  */
+      char const *p = m->args;
+      char const *plim = p + m->argbytes;
+      char const *arg[4];
+      for (int i = 0; i < 4; i++)
+       {
+         arg[i] = p;
+         if (p < plim)
+           p += strlen (p) + 1;
+       }
+      printf (_(m->msgid), arg[0], arg[1], arg[2], arg[3]);
+      if (p < plim)
+       abort ();
       struct msg *next = m->next;
-      arg[0] = m->args;
-      for (i = 0;  i < 4;  i++)
-        arg[i + 1] = arg[i] + strlen (arg[i]) + 1;
-      printf (_(arg[0]), arg[1], arg[2], arg[3], arg[4]);
       free (m);
       m = next;
     }
-- 
2.31.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]