Re: A minor fix in nl-langinfo

[Mark H Weaver]

Mark H Weaver <address@hidden> writes:

> address@hidden (Ludovic Courtès) writes:
>
>> Eli Zaretskii <address@hidden> skribis:
>>
>>> Sorry, that missed one more instance.  Please use this patch instead.
>>>
>>> --- libguile/i18n.c~0       2014-08-08 17:05:57.262034100 +0300
>>> +++ libguile/i18n.c 2014-08-10 17:20:52.073000000 +0300
>>> @@ -1497,6 +1497,8 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
>>>      {
>>>  #ifdef USE_GNU_LOCALE_API
>>>        c_result = nl_langinfo_l (c_item, c_locale);
>>> +      if (c_result != NULL)
>>> +   c_result = strdup (c_result);
>>>        codeset = nl_langinfo_l (CODESET, c_locale);

So, I think we have to copy 'codeset' also, because it is not used until
after the locale mutex is released, at which point another thread could
call 'nl_langinfo*' and overwrite the static buffer that 'codeset'
points to.

Here's the patch I had in mind.  What do you think?

     Mark


--8<---------------cut here---------------start------------->8---
diff --git a/libguile/i18n.c b/libguile/i18n.c
index e38e560..411f38a 100644
--- a/libguile/i18n.c
+++ b/libguile/i18n.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Free Software 
Foundation, Inc.
+/* Copyright (C) 2006-2014 Free Software Foundation, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License
@@ -1465,6 +1465,13 @@ SCM_DEFINE (scm_locale_string_to_inexact, 
"locale-string->inexact",
    Note: We don't use Gnulib's `nl_langinfo' module because it's currently not
    as complete as the compatibility hacks in `i18n.scm'.  */
 
+static char *
+copy_string_or_null (char *s)
+{
+  if (s != NULL)
+    s = strdup (s);
+  return s;
+}
 
 SCM_DEFINE (scm_nl_langinfo, "nl-langinfo", 1, 1, 0,
            (SCM item, SCM locale),
@@ -1496,8 +1503,8 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinfo", 1, 1, 0,
   if (c_locale != NULL)
     {
 #ifdef USE_GNU_LOCALE_API
-      c_result = nl_langinfo_l (c_item, c_locale);
-      codeset = nl_langinfo_l (CODESET, c_locale);
+      c_result = copy_string_or_null (nl_langinfo_l (c_item, c_locale));
+      codeset = copy_string_or_null (nl_langinfo_l (CODESET, c_locale));
 #else /* !USE_GNU_LOCALE_API */
       /* We can't use `RUN_IN_LOCALE_SECTION ()' here because the locale
         mutex is already taken.  */
@@ -1521,8 +1528,8 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinfo", 1, 1, 0,
        scm_locale_error (FUNC_NAME, lsec_err);
       else
        {
-         c_result = nl_langinfo (c_item);
-          codeset = nl_langinfo (CODESET);
+         c_result = copy_string_or_null (nl_langinfo (c_item));
+          codeset = copy_string_or_null (nl_langinfo (CODESET));
 
          restore_locale_settings (&lsec_prev_locale);
          free_locale_settings (&lsec_prev_locale);
@@ -1531,13 +1538,10 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinfo", 1, 1, 0,
     }
   else
     {
-      c_result = nl_langinfo (c_item);
-      codeset = nl_langinfo (CODESET);
+      c_result = copy_string_or_null (nl_langinfo (c_item));
+      codeset = copy_string_or_null (nl_langinfo (CODESET));
     }
 
-  if (c_result != NULL)
-    c_result = strdup (c_result);
-
   unlock_locale_mutex ();
 
   if (c_result == NULL)
@@ -1669,6 +1673,9 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinfo", 1, 1, 0,
        }
     }
 
+  if (codeset != NULL)
+    free (codeset);
+
   return result;
 }
 #undef FUNC_NAME
--8<---------------cut here---------------end--------------->8---