]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
getopt: clean up error reporting
authorZack Weinberg <zackw@panix.com>
Sat, 1 Apr 2017 14:17:44 +0000 (10:17 -0400)
committerZack Weinberg <zackw@panix.com>
Fri, 7 Apr 2017 11:48:57 +0000 (07:48 -0400)
getopt can print a whole bunch of error messages, and when used
standalone (from gnulib) it uses fprintf to do that.  But fprintf is a
cancellation point and getopt isn't, and also applying fprintf to a
stream in wide-character mode is not allowed.

glibc has an internal function called __fxprintf that writes a narrow
format string to a stream regardless of mode, but it only handles
ASCII format strings, and it's still a cancellation point.  getopt's
messages are translated, so they might not be ASCII.  So getopt has an
error message to an asprintf buffer, monkeys with internal flag bits
on stderr to disable cancellation, and then calls
__fxprintf(stderr, "%s", buffer).  There isn't even a helper function,
the code is duplicated every time.

This patch fixes __fxprintf to handle arbitrary multibyte format
strings, and adds a variant __fxprintf_nocancel that does the same
thing but also isn't a cancellation point.  (It still _works_ by
monkeying with internal flag bits on the FILE, but that's not really a
layering violation for code in stdio-common.)  All of the #ifdef _LIBC
blocks can then be reduced to their standalone versions with a little
help from some macros at the top of the file.

I also wrote a test case to verify that getopt really isn't a
cancellation point, and I'm glad I did, because it found two bugs, one
of which wasn't even to do with cancellation (see previous patch).

* stdio-common/fxprintf.c (__fxprintf_nocancel): New function.
(locked_vfxprintf): New helper function. Handle arbitrary
multibyte strings, not just ASCII.
* include/stdio.h: Declare __fxprintf_nocancel.

* posix/getopt.c: When _LIBC is defined, define fprintf to
__fxprintf_nocancel, flockfile to _IO_flockfile, and
funlockfile to _IO_funlockfile.  When neither _LIBC nor
_POSIX_THREAD_SAFE_FUNCTIONS is defined, define flockfile and
funlockfile as no-ops.  (_getopt_internal_r): Remove all
internal #ifdef _LIBC blocks; the standalone error-printing
code can now be used for libc as well.  Add an
flockfile/funlockfile pair around one case where the error
message is printed in several chunks.  Don't use fputc.

* posix/tst-getopt-cancel.c: New test.
* posix/Makefile: Run it.

ChangeLog
include/stdio.h
posix/Makefile
posix/getopt.c
posix/tst-getopt-cancel.c [new file with mode: 0644]
stdio-common/fxprintf.c

index 48e5b036d949947e43e6b778de34a8a3c9f9546a..d45be5fedd01a2786bea297cd0827a821f82bb26 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,21 @@
 2017-04-07  Zack Weinberg  <zackw@panix.com>
 
+       * stdio-common/fxprintf.c (__fxprintf_nocancel): New function.
+       (locked_vfxprintf): New helper function. Handle arbitrary
+       multibyte strings, not just ASCII.
+       * include/stdio.h: Declare __fxprintf_nocancel.
+       * posix/getopt.c: When _LIBC is defined, define fprintf to
+       __fxprintf_nocancel, flockfile to _IO_flockfile, and funlockfile
+       to _IO_funlockfile.  When neither _LIBC nor
+       _POSIX_THREAD_SAFE_FUNCTIONS is defined, define flockfile and
+       funlockfile as no-ops.
+       (_getopt_internal_r): Remove all internal #ifdef _LIBC blocks;
+       the standalone error-printing code can now be used for libc as
+       well.  Add an flockfile/funlockfile pair around one case where
+       the error message is printed in several chunks.  Don't use fputc.
+       * posix/tst-getopt-cancel.c: New test.
+       * posix/Makefile: Run it.
+
        * posix/getopt.c (_getopt_internal_r): Don't increment
        d->optind a second time when reporting ambiguous -W options.
 
index 17b5a050767bcff278bb056441c5e7b84e5c60fd..4e7cfa1be534088ae2707772fdd8924699efc0cf 100644 (file)
@@ -114,6 +114,8 @@ extern wint_t __getwc_unlocked (FILE *__fp);
 
 extern int __fxprintf (FILE *__fp, const char *__fmt, ...)
      __attribute__ ((__format__ (__printf__, 2, 3)));
+extern int __fxprintf_nocancel (FILE *__fp, const char *__fmt, ...)
+     __attribute__ ((__format__ (__printf__, 2, 3)));
 
 extern const char *const _sys_errlist_internal[] attribute_hidden;
 extern int _sys_nerr_internal attribute_hidden;
index efcbeff96a56d9f063340aac3e85148f9f03e50d..8e29eea5aeaf85f9f85e2e37b7041d1e46f71e75 100644 (file)
@@ -97,6 +97,9 @@ ifeq (yes,$(build-shared))
 test-srcs      := globtest
 tests           += wordexp-test tst-exec tst-spawn tst-spawn2 tst-spawn3
 endif
+ifeq (yesyes,$(build-shared)$(have-thread-library))
+tests          += tst-getopt-cancel
+endif
 tests-static   = tst-exec-static tst-spawn-static
 tests          += $(tests-static)
 others         := getconf
@@ -254,6 +257,8 @@ ptestcases.h: PTESTS PTESTS2C.sed
        LC_ALL=C sed -f PTESTS2C.sed < $< > $@T
        mv -f $@T $@
 
+$(objpfx)tst-getopt-cancel: $(shared-thread-library)
+
 test-xfail-annexc = yes
 $(objpfx)annexc.out: $(objpfx)annexc
        $(dir $<)$(notdir $<) '$(CC)' \
index e616aa6e4da168e18f322adf45e539ca34491839..248fe5b03a4f26e6323b6b3898e67e507a340614 100644 (file)
 
 #include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
 #include <string.h>
+#include <unistd.h>
 
 #ifdef _LIBC
+/* When used as part of glibc, error printing must be done differently
+   for standards compliance.  getopt is not a cancellation point, so
+   it must not call functions that are, and it is specified by an
+   older standard than stdio locking, so it must not refer to
+   functions in the "user namespace" related to stdio locking.
+   Finally, it must use glibc's internal message translation so that
+   the messages are looked up in the proper text domain.  */
 # include <libintl.h>
+# define fprintf __fxprintf_nocancel
+# define flockfile(fp) _IO_flockfile (fp)
+# define funlockfile(fp) _IO_funlockfile (fp)
 #else
 # include "gettext.h"
 # define _(msgid) gettext (msgid)
+/* When used standalone, flockfile and funlockfile might not be
+   available.  */
+# ifndef _POSIX_THREAD_SAFE_FUNCTIONS
+#  define flockfile(fp) /* nop */
+#  define funlockfile(fp) /* nop */
+# endif
 #endif
 
 /* This implementation of 'getopt' has three modes for handling
@@ -98,7 +114,6 @@ int optopt = '?';
 /* Keep a global copy of all internal members of getopt_data.  */
 
 static struct _getopt_data getopt_data;
-
 \f
 /* Exchange two adjacent subsequences of ARGV.
    One subsequence is elements [first_nonopt,last_nonopt)
@@ -271,7 +286,7 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
   if (d->optind == 0 || !d->__initialized)
     {
       if (d->optind == 0)
-       d->optind = 1;  /* Don't scan ARGV[0], the program name.  */
+       d->optind = 1;  /* Don't scan ARGV[0], the program name.  */
       optstring = _getopt_initialize (argc, argv, optstring, d,
                                      posixly_correct);
       d->__initialized = 1;
@@ -441,42 +456,8 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
              first.next = ambig_list;
              ambig_list = &first;
 
-#if defined _LIBC
-             char *buf = NULL;
-             size_t buflen = 0;
-
-             FILE *fp = __open_memstream (&buf, &buflen);
-             if (fp != NULL)
-               {
-                 fprintf (fp,
-                          _("%s: option '%s' is ambiguous; possibilities:"),
-                          argv[0], argv[d->optind]);
-
-                 do
-                   {
-                     fprintf (fp, " '--%s'", ambig_list->p->name);
-                     ambig_list = ambig_list->next;
-                   }
-                 while (ambig_list != NULL);
-
-                 fputc_unlocked ('\n', fp);
-
-                 if (__glibc_likely (fclose (fp) != EOF))
-                   {
-                     _IO_flockfile (stderr);
-
-                     int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-                     ((_IO_FILE *) stderr)->_flags2 |= _IO_FLAGS2_NOTCANCEL;
+             flockfile (stderr);
 
-                     __fxprintf (NULL, "%s", buf);
-
-                     ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-                     _IO_funlockfile (stderr);
-
-                     free (buf);
-                   }
-               }
-#else
              fprintf (stderr,
                       _("%s: option '%s' is ambiguous; possibilities:"),
                       argv[0], argv[d->optind]);
@@ -487,8 +468,11 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
                }
              while (ambig_list != NULL);
 
-             fputc ('\n', stderr);
-#endif
+             /* This must use 'fprintf' even though it's only printing a
+                single character, so that it goes through __fxprintf_nocancel
+                when compiled as part of glibc.  */
+             fprintf (stderr, "\n");
+             funlockfile (stderr);
            }
          d->__nextchar += strlen (d->__nextchar);
          d->optind++;
@@ -510,57 +494,17 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
                {
                  if (print_errors)
                    {
-#if defined _LIBC
-                     char *buf;
-                     int n;
-#endif
-
                      if (argv[d->optind - 1][1] == '-')
-                       {
-                         /* --option */
-#if defined _LIBC
-                         n = __asprintf (&buf, _("\
-%s: option '--%s' doesn't allow an argument\n"),
-                                         argv[0], pfound->name);
-#else
-                         fprintf (stderr, _("\
+                       /* --option */
+                       fprintf (stderr, _("\
 %s: option '--%s' doesn't allow an argument\n"),
-                                  argv[0], pfound->name);
-#endif
-                       }
+                                argv[0], pfound->name);
                      else
-                       {
-                         /* +option or -option */
-#if defined _LIBC
-                         n = __asprintf (&buf, _("\
-%s: option '%c%s' doesn't allow an argument\n"),
-                                         argv[0], argv[d->optind - 1][0],
-                                         pfound->name);
-#else
-                         fprintf (stderr, _("\
+                       /* +option or -option */
+                       fprintf (stderr, _("\
 %s: option '%c%s' doesn't allow an argument\n"),
-                                  argv[0], argv[d->optind - 1][0],
-                                  pfound->name);
-#endif
-                       }
-
-#if defined _LIBC
-                     if (n >= 0)
-                       {
-                         _IO_flockfile (stderr);
-
-                         int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-                         ((_IO_FILE *) stderr)->_flags2
-                           |= _IO_FLAGS2_NOTCANCEL;
-
-                         __fxprintf (NULL, "%s", buf);
-
-                         ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-                         _IO_funlockfile (stderr);
-
-                         free (buf);
-                       }
-#endif
+                                argv[0], argv[d->optind - 1][0],
+                                pfound->name);
                    }
 
                  d->__nextchar += strlen (d->__nextchar);
@@ -576,33 +520,10 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
              else
                {
                  if (print_errors)
-                   {
-#if defined _LIBC
-                     char *buf;
-
-                     if (__asprintf (&buf, _("\
-%s: option '--%s' requires an argument\n"),
-                                     argv[0], pfound->name) >= 0)
-                       {
-                         _IO_flockfile (stderr);
-
-                         int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-                         ((_IO_FILE *) stderr)->_flags2
-                           |= _IO_FLAGS2_NOTCANCEL;
-
-                         __fxprintf (NULL, "%s", buf);
-
-                         ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-                         _IO_funlockfile (stderr);
+                   fprintf (stderr,
+                            _("%s: option '--%s' requires an argument\n"),
+                            argv[0], pfound->name);
 
-                         free (buf);
-                       }
-#else
-                     fprintf (stderr,
-                              _("%s: option '--%s' requires an argument\n"),
-                              argv[0], pfound->name);
-#endif
-                   }
                  d->__nextchar += strlen (d->__nextchar);
                  d->optopt = pfound->val;
                  return optstring[0] == ':' ? ':' : '?';
@@ -628,50 +549,14 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
        {
          if (print_errors)
            {
-#if defined _LIBC
-             char *buf;
-             int n;
-#endif
-
              if (argv[d->optind][1] == '-')
-               {
-                 /* --option */
-#if defined _LIBC
-                 n = __asprintf (&buf, _("%s: unrecognized option '--%s'\n"),
-                                 argv[0], d->__nextchar);
-#else
-                 fprintf (stderr, _("%s: unrecognized option '--%s'\n"),
-                          argv[0], d->__nextchar);
-#endif
-               }
+               /* --option */
+               fprintf (stderr, _("%s: unrecognized option '--%s'\n"),
+                        argv[0], d->__nextchar);
              else
-               {
-                 /* +option or -option */
-#if defined _LIBC
-                 n = __asprintf (&buf, _("%s: unrecognized option '%c%s'\n"),
-                                 argv[0], argv[d->optind][0], d->__nextchar);
-#else
-                 fprintf (stderr, _("%s: unrecognized option '%c%s'\n"),
-                          argv[0], argv[d->optind][0], d->__nextchar);
-#endif
-               }
-
-#if defined _LIBC
-             if (n >= 0)
-               {
-                 _IO_flockfile (stderr);
-
-                 int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-                 ((_IO_FILE *) stderr)->_flags2 |= _IO_FLAGS2_NOTCANCEL;
-
-                 __fxprintf (NULL, "%s", buf);
-
-                 ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-                 _IO_funlockfile (stderr);
-
-                 free (buf);
-               }
-#endif
+               /* +option or -option */
+               fprintf (stderr, _("%s: unrecognized option '%c%s'\n"),
+                        argv[0], argv[d->optind][0], d->__nextchar);
            }
          d->__nextchar = (char *) "";
          d->optind++;
@@ -693,36 +578,7 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
     if (temp == NULL || c == ':' || c == ';')
       {
        if (print_errors)
-         {
-#if defined _LIBC
-           char *buf;
-           int n;
-#endif
-
-#if defined _LIBC
-           n = __asprintf (&buf, _("%s: invalid option -- '%c'\n"),
-                           argv[0], c);
-#else
-           fprintf (stderr, _("%s: invalid option -- '%c'\n"), argv[0], c);
-#endif
-
-#if defined _LIBC
-           if (n >= 0)
-             {
-               _IO_flockfile (stderr);
-
-               int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-               ((_IO_FILE *) stderr)->_flags2 |= _IO_FLAGS2_NOTCANCEL;
-
-               __fxprintf (NULL, "%s", buf);
-
-               ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-               _IO_funlockfile (stderr);
-
-               free (buf);
-             }
-#endif
-         }
+         fprintf (stderr, _("%s: invalid option -- '%c'\n"), argv[0], c);
        d->optopt = c;
        return '?';
       }
@@ -751,32 +607,10 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
        else if (d->optind == argc)
          {
            if (print_errors)
-             {
-#if defined _LIBC
-               char *buf;
-
-               if (__asprintf (&buf,
-                               _("%s: option requires an argument -- '%c'\n"),
-                               argv[0], c) >= 0)
-                 {
-                   _IO_flockfile (stderr);
-
-                   int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-                   ((_IO_FILE *) stderr)->_flags2 |= _IO_FLAGS2_NOTCANCEL;
-
-                   __fxprintf (NULL, "%s", buf);
-
-                   ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-                   _IO_funlockfile (stderr);
+             fprintf (stderr,
+                      _("%s: option requires an argument -- '%c'\n"),
+                      argv[0], c);
 
-                   free (buf);
-                 }
-#else
-               fprintf (stderr,
-                        _("%s: option requires an argument -- '%c'\n"),
-                        argv[0], c);
-#endif
-             }
            d->optopt = c;
            if (optstring[0] == ':')
              c = ':';
@@ -825,30 +659,9 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
        if (ambig && !exact)
          {
            if (print_errors)
-             {
-#if defined _LIBC
-               char *buf;
-
-               if (__asprintf (&buf, _("%s: option '-W %s' is ambiguous\n"),
-                               argv[0], d->optarg) >= 0)
-                 {
-                   _IO_flockfile (stderr);
-
-                   int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-                   ((_IO_FILE *) stderr)->_flags2 |= _IO_FLAGS2_NOTCANCEL;
+             fprintf (stderr, _("%s: option '-W %s' is ambiguous\n"),
+                      argv[0], d->optarg);
 
-                   __fxprintf (NULL, "%s", buf);
-
-                   ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-                   _IO_funlockfile (stderr);
-
-                   free (buf);
-                 }
-#else
-               fprintf (stderr, _("%s: option '-W %s' is ambiguous\n"),
-                        argv[0], d->optarg);
-#endif
-             }
            d->__nextchar += strlen (d->__nextchar);
            return '?';
          }
@@ -864,33 +677,9 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
                else
                  {
                    if (print_errors)
-                     {
-#if defined _LIBC
-                       char *buf;
-
-                       if (__asprintf (&buf, _("\
-%s: option '-W %s' doesn't allow an argument\n"),
-                                       argv[0], pfound->name) >= 0)
-                         {
-                           _IO_flockfile (stderr);
-
-                           int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-                           ((_IO_FILE *) stderr)->_flags2
-                             |= _IO_FLAGS2_NOTCANCEL;
-
-                           __fxprintf (NULL, "%s", buf);
-
-                           ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-                           _IO_funlockfile (stderr);
-
-                           free (buf);
-                         }
-#else
-                       fprintf (stderr, _("\
+                     fprintf (stderr, _("\
 %s: option '-W %s' doesn't allow an argument\n"),
-                                argv[0], pfound->name);
-#endif
-                     }
+                              argv[0], pfound->name);
 
                    d->__nextchar += strlen (d->__nextchar);
                    return '?';
@@ -903,33 +692,10 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
                else
                  {
                    if (print_errors)
-                     {
-#if defined _LIBC
-                       char *buf;
-
-                       if (__asprintf (&buf, _("\
+                     fprintf (stderr, _("\
 %s: option '-W %s' requires an argument\n"),
-                                       argv[0], pfound->name) >= 0)
-                         {
-                           _IO_flockfile (stderr);
-
-                           int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-                           ((_IO_FILE *) stderr)->_flags2
-                             |= _IO_FLAGS2_NOTCANCEL;
-
-                           __fxprintf (NULL, "%s", buf);
-
-                           ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-                           _IO_funlockfile (stderr);
+                              argv[0], pfound->name);
 
-                           free (buf);
-                         }
-#else
-                       fprintf (stderr, _("\
-%s: option '-W %s' requires an argument\n"),
-                                argv[0], pfound->name);
-#endif
-                     }
                    d->__nextchar += strlen (d->__nextchar);
                    return optstring[0] == ':' ? ':' : '?';
                  }
@@ -978,32 +744,10 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
            else if (d->optind == argc)
              {
                if (print_errors)
-                 {
-#if defined _LIBC
-                   char *buf;
+                 fprintf (stderr,
+                          _("%s: option requires an argument -- '%c'\n"),
+                          argv[0], c);
 
-                   if (__asprintf (&buf, _("\
-%s: option requires an argument -- '%c'\n"),
-                                   argv[0], c) >= 0)
-                     {
-                       _IO_flockfile (stderr);
-
-                       int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-                       ((_IO_FILE *) stderr)->_flags2 |= _IO_FLAGS2_NOTCANCEL;
-
-                       __fxprintf (NULL, "%s", buf);
-
-                       ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-                       _IO_funlockfile (stderr);
-
-                       free (buf);
-                     }
-#else
-                   fprintf (stderr,
-                            _("%s: option requires an argument -- '%c'\n"),
-                            argv[0], c);
-#endif
-                 }
                d->optopt = c;
                if (optstring[0] == ':')
                  c = ':';
diff --git a/posix/tst-getopt-cancel.c b/posix/tst-getopt-cancel.c
new file mode 100644 (file)
index 0000000..594596a
--- /dev/null
@@ -0,0 +1,284 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* fprintf is a cancellation point, but getopt is not supposed to be a
+   cancellation point, even when it prints error messages.  */
+
+/* Note: getopt.h must be included first in this file, so we get the
+   GNU getopt rather than the POSIX one.  */
+#include <getopt.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <fcntl.h>
+#include <pthread.h>
+#include <unistd.h>
+
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/xthread.h>
+
+static bool
+check_stderr (bool expect_errmsg, FILE *stderr_trapped)
+{
+  static char *lineptr = 0;
+  static size_t linesz = 0;
+
+  bool got_errmsg = false;
+  rewind (stderr_trapped);
+  while (getline (&lineptr, &linesz, stderr_trapped) > 0)
+    {
+      got_errmsg = true;
+      fputs (lineptr, stdout);
+    }
+  rewind (stderr_trapped);
+  ftruncate (fileno (stderr_trapped), 0);
+  return got_errmsg == expect_errmsg;
+}
+
+struct test_short
+{
+  const char *label;
+  const char *opts;
+  const char *const argv[8];
+  int argc;
+  bool expect_errmsg;
+};
+
+struct test_long
+{
+  const char *label;
+  const char *opts;
+  const struct option longopts[4];
+  const char *const argv[8];
+  int argc;
+  bool expect_errmsg;
+};
+
+#define DEFINE_TEST_DRIVER(test_type, getopt_call)                     \
+  struct test_type##_tdata                                             \
+  {                                                                    \
+    pthread_mutex_t *sync;                                             \
+    const struct test_type *tcase;                                     \
+    bool ok;                                                           \
+  };                                                                   \
+                                                                       \
+  static void *                                                                \
+  test_type##_threadproc (void *data)                                  \
+  {                                                                    \
+    struct test_type##_tdata *tdata = data;                            \
+    const struct test_type *tc = tdata->tcase;                         \
+                                                                       \
+    xpthread_mutex_lock (tdata->sync);                                 \
+    xpthread_mutex_unlock (tdata->sync);                               \
+                                                                       \
+    /* At this point, this thread has a cancellation pending.          \
+       We should still be able to get all the way through a getopt     \
+       loop without being cancelled.                                   \
+       Setting optind to 0 forces getopt to reinitialize itself.  */   \
+    optind = 0;                                                                \
+    opterr = 1;                                                                \
+    optopt = 0;                                                                \
+    while (getopt_call != -1)                                          \
+      ;                                                                        \
+    tdata->ok = true;                                                  \
+                                                                       \
+    pthread_testcancel();                                              \
+    return 0;                                                          \
+  }                                                                    \
+                                                                       \
+  static bool                                                          \
+  do_##test_type (const struct test_type *tcase, FILE *stderr_trapped) \
+  {                                                                    \
+    pthread_mutex_t sync;                                              \
+    struct test_type##_tdata tdata;                                    \
+                                                                       \
+    printf("begin: %s\n", tcase->label);                               \
+                                                                       \
+    xpthread_mutex_init (&sync, 0);                                    \
+    xpthread_mutex_lock (&sync);                                       \
+                                                                       \
+    tdata.sync = &sync;                                                        \
+    tdata.tcase = tcase;                                               \
+    tdata.ok = false;                                                  \
+                                                                       \
+    pthread_t thr = xpthread_create (0, test_type##_threadproc,                \
+                                    (void *)&tdata);                   \
+    xpthread_cancel (thr);                                             \
+    xpthread_mutex_unlock (&sync);                                     \
+    void *rv = xpthread_join (thr);                                    \
+                                                                       \
+    xpthread_mutex_destroy (&sync);                                    \
+                                                                       \
+    bool ok = true;                                                    \
+    if (!check_stderr (tcase->expect_errmsg, stderr_trapped))          \
+      {                                                                        \
+       ok = false;                                                     \
+       printf("FAIL: %s: stderr not as expected\n", tcase->label);     \
+      }                                                                        \
+    if (!tdata.ok)                                                     \
+      {                                                                        \
+       ok = false;                                                     \
+       printf("FAIL: %s: did not complete loop\n", tcase->label);      \
+      }                                                                        \
+    if (rv != PTHREAD_CANCELED)                                                \
+      {                                                                        \
+       ok = false;                                                     \
+       printf("FAIL: %s: thread was not cancelled\n", tcase->label);   \
+      }                                                                        \
+    if (ok)                                                            \
+      printf ("pass: %s\n", tcase->label);                             \
+    return ok;                                                         \
+  }
+
+DEFINE_TEST_DRIVER (test_short,
+                   getopt (tc->argc, (char *const *)tc->argv, tc->opts))
+DEFINE_TEST_DRIVER (test_long,
+                   getopt_long (tc->argc, (char *const *)tc->argv,
+                                tc->opts, tc->longopts, 0))
+
+/* Caution: all option strings must begin with a '+' or '-' so that
+   getopt does not attempt to permute the argument vector (which is in
+   read-only memory).  */
+const struct test_short tests_short[] = {
+  { "no errors",
+    "+ab:c", { "program", "-ac", "-b", "x", 0 }, 4, false },
+  { "invalid option",
+    "+ab:c", { "program", "-d", 0 },            2, true },
+  { "missing argument",
+    "+ab:c", { "program", "-b", 0 },            2, true },
+  { 0 }
+};
+
+const struct test_long tests_long[] = {
+  { "no errors (long)",
+    "+ab:c", { { "alpha",   no_argument,       0, 'a' },
+              { "bravo",   required_argument, 0, 'b' },
+              { "charlie", no_argument,       0, 'c' },
+              { 0 } },
+    { "program", "-a", "--charlie", "--bravo=x", 0 }, 4, false },
+
+  { "invalid option (long)",
+    "+ab:c", { { "alpha",   no_argument,       0, 'a' },
+              { "bravo",   required_argument, 0, 'b' },
+              { "charlie", no_argument,       0, 'c' },
+              { 0 } },
+    { "program", "-a", "--charlie", "--dingo", 0 }, 4, true },
+
+  { "unwanted argument",
+    "+ab:c", { { "alpha",   no_argument,       0, 'a' },
+              { "bravo",   required_argument, 0, 'b' },
+              { "charlie", no_argument,       0, 'c' },
+              { 0 } },
+    { "program", "-a", "--charlie=dingo", "--bravo=x", 0 }, 4, true },
+
+  { "missing argument",
+    "+ab:c", { { "alpha",   no_argument,       0, 'a' },
+              { "bravo",   required_argument, 0, 'b' },
+              { "charlie", no_argument,       0, 'c' },
+              { 0 } },
+    { "program", "-a", "--charlie", "--bravo", 0 }, 4, true },
+
+  { "ambiguous options",
+    "+uvw", { { "veni", no_argument, 0, 'u' },
+             { "vedi", no_argument, 0, 'v' },
+             { "veci", no_argument, 0, 'w' } },
+    { "program", "--ve", 0 }, 2, true },
+
+  { "no errors (long W)",
+    "+ab:cW;", { { "alpha",   no_argument,      0, 'a' },
+                { "bravo",   required_argument, 0, 'b' },
+                { "charlie", no_argument,       0, 'c' },
+                { 0 } },
+    { "program", "-a", "-W", "charlie", "-W", "bravo=x", 0 }, 6, false },
+
+  { "missing argument (W itself)",
+    "+ab:cW;", { { "alpha",   no_argument,      0, 'a' },
+                { "bravo",   required_argument, 0, 'b' },
+                { "charlie", no_argument,       0, 'c' },
+                { 0 } },
+    { "program", "-a", "-W", "charlie", "-W", 0 }, 5, true },
+
+  { "missing argument (W longopt)",
+    "+ab:cW;", { { "alpha",   no_argument,      0, 'a' },
+                { "bravo",   required_argument, 0, 'b' },
+                { "charlie", no_argument,       0, 'c' },
+                { 0 } },
+    { "program", "-a", "-W", "charlie", "-W", "bravo", 0 }, 6, true },
+
+  { "unwanted argument (W longopt)",
+    "+ab:cW;", { { "alpha",   no_argument,      0, 'a' },
+                { "bravo",   required_argument, 0, 'b' },
+                { "charlie", no_argument,       0, 'c' },
+                { 0 } },
+    { "program", "-a", "-W", "charlie=dingo", "-W", "bravo=x", 0 }, 6, true },
+
+  { "ambiguous options (W)",
+    "+uvwW;", { { "veni", no_argument, 0, 'u' },
+               { "vedi", no_argument, 0, 'v' },
+               { "veci", no_argument, 0, 'w' } },
+    { "program", "-W", "ve", 0 }, 3, true },
+
+  { 0 }
+};
+
+static int
+do_test (void)
+{
+  int stderr_trap = create_temp_file ("stderr", 0);
+  if (stderr_trap < 0)
+    {
+      perror ("create_temp_file");
+      return 1;
+    }
+  FILE *stderr_trapped = fdopen(stderr_trap, "r+");
+  if (!stderr_trapped)
+    {
+      perror ("fdopen");
+      return 1;
+    }
+  int old_stderr = dup (fileno (stderr));
+  if (old_stderr < 0)
+    {
+      perror ("dup");
+      return 1;
+    }
+  if (dup2 (stderr_trap, 2) < 0)
+    {
+      perror ("dup2");
+      return 1;
+    }
+  rewind (stderr);
+
+  bool success = true;
+
+  for (const struct test_short *tcase = tests_short; tcase->label; tcase++)
+    success = do_test_short (tcase, stderr_trapped) && success;
+
+  for (const struct test_long *tcase = tests_long; tcase->label; tcase++)
+    success = do_test_long (tcase, stderr_trapped) && success;
+
+  dup2 (old_stderr, 2);
+  close (old_stderr);
+  fclose (stderr_trapped);
+
+  return success ? 0 : 1;
+}
+
+#include <support/test-driver.c>
index 5ed0bb033bc614f17edd60c70bd7098fcd3b0333..82d4f2e235c93c5c0d482893ce86385738da160e 100644 (file)
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <assert.h>
-#include <ctype.h>
 #include <stdarg.h>
 #include <stdio.h>
-#include <wchar.h>
+#include <stdlib.h>
 #include <string.h>
+#include <wchar.h>
 #include <libioP.h>
 
+static int
+locked_vfxprintf (FILE *fp, const char *fmt, va_list ap)
+{
+  if (_IO_fwide (fp, 0) <= 0)
+    return _IO_vfprintf (fp, fmt, ap);
+
+  /* We must convert the narrow format string to a wide one.
+     Each byte can produce at most one wide character.  */
+  wchar_t *wfmt;
+  mbstate_t mbstate;
+  int res;
+  int used_malloc = 0;
+  size_t len = strlen (fmt) + 1;
+
+  if (__glibc_unlikely (len > SIZE_MAX / sizeof (wchar_t)))
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+  if (__libc_use_alloca (len * sizeof (wchar_t)))
+    wfmt = alloca (len * sizeof (wchar_t));
+  else if ((wfmt = malloc (len * sizeof (wchar_t))) == NULL)
+    return -1;
+  else
+    used_malloc = 1;
+
+  memset (&mbstate, 0, sizeof mbstate);
+  res = __mbsrtowcs (wfmt, &fmt, len, &mbstate);
+
+  if (res != -1)
+    res = _IO_vfwprintf (fp, wfmt, ap);
+
+  if (used_malloc)
+    free (wfmt);
+
+  return res;
+}
 
 int
 __fxprintf (FILE *fp, const char *fmt, ...)
@@ -33,23 +69,31 @@ __fxprintf (FILE *fp, const char *fmt, ...)
 
   va_list ap;
   va_start (ap, fmt);
+  _IO_flockfile (fp);
 
-  int res;
-  if (_IO_fwide (fp, 0) > 0)
-    {
-      size_t len = strlen (fmt) + 1;
-      wchar_t wfmt[len];
-      for (size_t i = 0; i < len; ++i)
-       {
-         assert (isascii (fmt[i]));
-         wfmt[i] = fmt[i];
-       }
-      res = __vfwprintf (fp, wfmt, ap);
-    }
-  else
-    res = _IO_vfprintf (fp, fmt, ap);
+  int res = locked_vfxprintf (fp, fmt, ap);
 
+  _IO_funlockfile (fp);
   va_end (ap);
+  return res;
+}
 
+int
+__fxprintf_nocancel (FILE *fp, const char *fmt, ...)
+{
+  if (fp == NULL)
+    fp = stderr;
+
+  va_list ap;
+  va_start (ap, fmt);
+  _IO_flockfile (fp);
+  int save_flags2 = ((_IO_FILE *)fp)->_flags2;
+  ((_IO_FILE *)fp)->_flags2 |= _IO_FLAGS2_NOTCANCEL;
+
+  int res = locked_vfxprintf (fp, fmt, ap);
+
+  ((_IO_FILE *)fp)->_flags2 = save_flags2;
+  _IO_funlockfile (fp);
+  va_end (ap);
   return res;
 }