This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] locale: Add LOCPATH diagnostics to the locale program


On 4/23/19 11:10 AM, Florian Weimer wrote:
The implementation of quote_string is based on support_quote_blob.


LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>


2019-04-23  Florian Weimer  <fweimer@redhat.com>

	locale: Add LOCPATH diagnostics to the locale program.
	* locale/programs/locale.c (setlocale_failed): New variable.
	(try_setlocale): New function.
	(quote_string): Likewise.
	(setlocale_diagnostics): Likewise.
	(main): Call try_setlocale instead of setlocale.  Call
	setlocale_diagnostics.
	* locale/Makefile (tests-special): Add tst-locale-locpath.out.
	(tst-locale-locpath.out): New target.
	* locale/tst-locale-locpath.sh: New file.

diff --git a/locale/Makefile b/locale/Makefile
index 764e751c36..6822b795dd 100644
--- a/locale/Makefile
+++ b/locale/Makefile
@@ -28,6 +28,7 @@ routines	= setlocale findlocale loadlocale loadarchive \
  		  localeconv nl_langinfo nl_langinfo_l mb_cur_max \
  		  newlocale duplocale freelocale uselocale
  tests		= tst-C-locale tst-locname tst-duplocale
+tests-special	= $(objpfx)tst-locale-locpath.out

OK.

  categories	= ctype messages monetary numeric time paper name \
  		  address telephone measurement identification collate
  aux		= $(categories:%=lc-%) $(categories:%=C-%) SYS_libc C_name \
@@ -107,3 +108,7 @@ cpp-srcs-left := $(localedef-modules) $(localedef-aux) $(locale-modules) \
  		 $(lib-modules)
  lib := locale-programs
  include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
+
+$(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale
+	$(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)' > $@; \
+	$(evaluate-test)

OK.

diff --git a/locale/programs/locale.c b/locale/programs/locale.c
index 8af0d78a77..6eae3175bb 100644
--- a/locale/programs/locale.c
+++ b/locale/programs/locale.c
@@ -173,6 +173,9 @@ static int write_archive_locales (void **all_datap, char *linebuf);
  static void write_charmaps (void);
  static void show_locale_vars (void);
  static void show_info (const char *name);
+static void try_setlocale (int category, const char *category_name);
+static char *quote_string (const char *input);
+static void setlocale_diagnostics (void);

OK.

int
@@ -186,10 +189,8 @@ main (int argc, char *argv[])
/* Set locale. Do not set LC_ALL because the other categories must
       not be affected (according to POSIX.2).  */
-  if (setlocale (LC_CTYPE, "") == NULL)
-    error (0, errno, gettext ("Cannot set LC_CTYPE to default locale"));
-  if (setlocale (LC_MESSAGES, "") == NULL)
-    error (0, errno, gettext ("Cannot set LC_MESSAGES to default locale"));
+  try_setlocale (LC_CTYPE, "LC_CTYPE");
+  try_setlocale (LC_MESSAGES, "LC_MESSAGES");

OK.

/* Initialize the message catalog. */
    textdomain (PACKAGE);
@@ -200,9 +201,8 @@ main (int argc, char *argv[])
    /* `-a' requests the names of all available locales.  */
    if (do_all != 0)
      {
-      if (setlocale (LC_COLLATE, "") == NULL)
-	error (0, errno,
-	       gettext ("Cannot set LC_COLLATE to default locale"));
+      setlocale_diagnostics ();
+      try_setlocale (LC_COLLATE, "LC_COLLATE");

OK.

        write_locales ();
        exit (EXIT_SUCCESS);
      }
@@ -211,14 +211,15 @@ main (int argc, char *argv[])
       used for the -f argument to localedef(1).  */
    if (do_charmaps != 0)
      {
+      setlocale_diagnostics ();

OK.

        write_charmaps ();
        exit (EXIT_SUCCESS);
      }
/* Specific information about the current locale are requested.
       Change to this locale now.  */
-  if (setlocale (LC_ALL, "") == NULL)
-    error (0, errno, gettext ("Cannot set LC_ALL to default locale"));
+  try_setlocale (LC_ALL, "LC_ALL");
+  setlocale_diagnostics ();

OK.

/* If no real argument is given we have to print the contents of the
       current locale definition variables.  These are LANG and the LC_*.  */
@@ -983,3 +984,121 @@ show_info (const char *name)
       For testing and perhaps advanced use allow some more symbols.  */
    locale_special (name, show_category_name, show_keyword_name);
  }
+
+/* Set to true by try_setlocale if setlocale fails.  Used by
+   setlocale_diagnostics.  */
+static bool setlocale_failed;

OK.

+
+/* Call setlocale, with non-fatal error reporting.  */

OK. Like this much better, lets you get through all the problems and then
print some better diagnostics.

+static void
+try_setlocale (int category, const char *category_name)
+{
+  if (setlocale (category, "") == NULL)
+    {
+      error (0, errno, gettext ("Cannot set %s to default locale"),
+	     category_name);
+      setlocale_failed = true;
+    }
+}
+
+/* Return a quoted version of the passed string, or NULL on error.  */
+static char *
+quote_string (const char *input)
+{
+  char *buffer;
+  size_t length;
+  FILE *stream = open_memstream (&buffer, &length);
+  if (stream == NULL)
+    return NULL;
+
+  while (true)
+    {
+      unsigned char ch = *input++;
+      if (ch == '\0')
+	break;
+
+      /* Use C backslash escapes for those control characters for
+         which they are defined.  */
+      switch (ch)
+        {
+          case '\a':
+            putc_unlocked ('\\', stream);
+            putc_unlocked ('a', stream);
+            break;
+          case '\b':
+            putc_unlocked ('\\', stream);
+            putc_unlocked ('b', stream);
+            break;
+          case '\f':
+            putc_unlocked ('\\', stream);
+            putc_unlocked ('f', stream);
+            break;
+          case '\n':
+            putc_unlocked ('\\', stream);
+            putc_unlocked ('n', stream);
+            break;
+          case '\r':
+            putc_unlocked ('\\', stream);
+            putc_unlocked ('r', stream);
+            break;
+          case '\t':
+            putc_unlocked ('\\', stream);
+            putc_unlocked ('t', stream);
+            break;
+          case '\v':
+            putc_unlocked ('\\', stream);
+            putc_unlocked ('v', stream);
+            break;
+          case '\\':
+          case '\'':
+          case '\"':
+            putc_unlocked ('\\', stream);
+            putc_unlocked (ch, stream);
+            break;
+        default:
+          if (ch < ' ' || ch > '~')
+            /* Use octal sequences because they are fixed width,
+               unlike hexadecimal sequences.  */
+            fprintf (stream, "\\%03o", ch);
+          else
+            putc_unlocked (ch, stream);
+        }
+    }
+
+  if (ferror (stream))
+    {
+      fclose (stream);
+      free (buffer);
+      return NULL;
+    }
+  if (fclose (stream) != 0)
+    {
+      free (buffer);
+      return NULL;
+    }
+
+  return buffer;
+}

OK.

+
+/* Print additional information if there was a setlocale error (during
+   try_setlocale).  */

OK. Great, we print LOCPATH value as part of the diagnostics to help users.

+static void
+setlocale_diagnostics (void)
+{
+  if (setlocale_failed)
+    {
+      const char *locpath = getenv ("LOCPATH");

OK.

+      if (locpath != NULL)
+	{
+	  char *quoted = quote_string (locpath);
+	  if (quoted != NULL)
+	    fprintf (stderr,
+		     gettext ("\
+warning: The LOCPATH variable is set to \"%s\"\n"),
+		     quoted);
+	  else
+	    fputs ("warning: The LOCPATH variable is set\n", stderr);
+	  free (quoted);
+	}
+    }
+}
diff --git a/locale/tst-locale-locpath.sh b/locale/tst-locale-locpath.sh
new file mode 100644
index 0000000000..b83de90a39
--- /dev/null
+++ b/locale/tst-locale-locpath.sh
@@ -0,0 +1,83 @@
+#!/bin/sh
+# Test that locale prints LOCPATH on failure.

OK.

+# Copyright (C) 2019 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/>.
+
+set -ex
+
+common_objpfx=$1
+test_wrapper_env=$2
+run_program_env=$3
+
+LIBPATH="$common_objpfx"
+
+testroot="${common_objpfx}locale/tst-locale-locpath-directory"
+cleanup () {
+    rm -rf "$testroot"
+}
+trap cleanup 0
+
+rm -rf "$testroot"
+mkdir -p $testroot
+
+unset LANG
+
+${test_wrapper_env} \
+${run_program_env} LC_ALL=invalid-locale LOCPATH=does-not-exist \
+${common_objpfx}elf/ld.so --library-path "$LIBPATH" \
+  "${common_objpfx}locale/locale" \
+  > "$testroot/stdout" 2> "$testroot/stderr"

OK, test running locale.

+
+echo "* standard error"
+cat "$testroot/stderr"
+echo "* standard output"
+cat "$testroot/stdout"
+
+cat > "$testroot/stderr-expected" <<EOF
+${common_objpfx}locale/locale: Cannot set LC_CTYPE to default locale: No such file or directory
+${common_objpfx}locale/locale: Cannot set LC_MESSAGES to default locale: No such file or directory
+${common_objpfx}locale/locale: Cannot set LC_ALL to default locale: No such file or directory
+warning: The LOCPATH variable is set to "does-not-exist"
+EOF
+
+cat > "$testroot/stdout-expected" <<EOF
+LANG=
+LC_CTYPE="invalid-locale"
+LC_NUMERIC="invalid-locale"
+LC_TIME="invalid-locale"
+LC_COLLATE="invalid-locale"
+LC_MONETARY="invalid-locale"
+LC_MESSAGES="invalid-locale"
+LC_PAPER="invalid-locale"
+LC_NAME="invalid-locale"
+LC_ADDRESS="invalid-locale"
+LC_TELEPHONE="invalid-locale"
+LC_MEASUREMENT="invalid-locale"
+LC_IDENTIFICATION="invalid-locale"
+LC_ALL=invalid-locale
+EOF
+
+errors=0
+if ! cmp -s "$testroot/stderr-expected" "$testroot/stderr" ; then
+    echo "error: standard error not correct"
+    errors=1
+fi
+if ! cmp -s "$testroot/stdout-expected" "$testroot/stdout" ; then
+    echo "error: standard output not correct"
+    errors=1
+fi
+exit $errors

OK.




--
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]