Fix multiple thinkos in newlocale

- Setting the categories strings in tmp_locale short-circuits
  __loadlocale.  Use a new_categories array instead, just as in
  _setlocale_r.

- If we have a base, copy over the *not* defined categories in
  category_mask in the first place.  Rearrange loop accordingly.

- Free base right in newlocale.

Signed-off by: Corinna Vinschen <corinna@vinschen.de>

newlib/libc/locale/newlocale.c

@@ -11,6 +11,7 @@ struct __locale_t *
 _newlocale_r (struct _reent *p, int category_mask, const char *locale,
 	      struct __locale_t *base)
 {
+  char new_categories[_LC_LAST][ENCODING_LEN + 1];
   struct __locale_t tmp_locale, *new_locale;
   int i;
 
@@ -35,61 +36,60 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
     return (struct __locale_t *) &__C_locale;
   /* Start with setting all values to the default locale values. */
   tmp_locale = __C_locale;
-  /* Fill out category strings. */
+  /* Fill out new category strings. */
-  if (!*locale)
-    {
-      for (i = 1; i < _LC_LAST; ++i)
-	if (((1 << i) & category_mask) != 0)
-	  {
-	    const char *env = __get_locale_env (p, i);
-	    if (strlen (env) > ENCODING_LEN)
-	      {
-		p->_errno = EINVAL;
-		return NULL;
-	      }
-	    strcpy (tmp_locale.categories[i], env);
-	  }
-    }
-  else
-    {
-      for (i = 1; i < _LC_LAST; ++i)
-	if (((1 << i) & category_mask) != 0)
-	  strcpy (tmp_locale.categories[i], locale);
-    }
-  /* Now go over all categories and set them. */
   for (i = 1; i < _LC_LAST; ++i)
     {
       if (((1 << i) & category_mask) != 0)
 	{
-	  /* Nothing to do for "C"/"POSIX" locale. */
+	  const char *cat = locale ?: __get_locale_env (p, i);
-	  if (!strcmp (tmp_locale.categories[i], "C")
+	  if (strlen (cat) > ENCODING_LEN)
-	      || !strcmp (tmp_locale.categories[i], "POSIX"))
-	    continue;
-	  /* If the new locale is the old locale, just copy it over. */
-	  if (base && !strcmp (base->categories[i], tmp_locale.categories[i]))
 	    {
-	      if (i == LC_CTYPE)
+	      p->_errno = EINVAL;
-		{
+	      return NULL;
-		  tmp_locale.wctomb = base->wctomb;
-		  tmp_locale.mbtowc = base->mbtowc;
-		  tmp_locale.cjk_lang = base->cjk_lang;
-		  tmp_locale.ctype_ptr - base->ctype_ptr;
-		}
-#ifdef __HAVE_LOCALE_INFO__
-	      tmp_locale.lc_cat[i].ptr = base->lc_cat[i].ptr;
-	      /* Mark the value as "has still to be copied".  We do this in
-		 two steps to simplify freeing new locale types in case of a
-		 subsequent error. */
-	      tmp_locale.lc_cat[i].buf = (void *) -1;
-#else
-	      if (i == LC_CTYPE)
-		strcpy (tmp_locale.ctype_codeset, base->ctype_codeset);
-	      else if (i == LC_MESSAGES)
-		strcpy (tmp_locale.message_codeset, base->message_codeset);
-#endif
 	    }
+	  strcpy (new_categories[i], cat);
+	}
+      else
+	strcpy (new_categories[i], base ? base->categories[i] : "C");
+    }
+  /* Now go over all categories and set them. */
+  for (i = 1; i < _LC_LAST; ++i)
+    {
+      /* If we have a base locale, and the category is not in category_mask
+	 or the new category is the base categroy, just copy over. */
+      if (base && (((1 << i) & category_mask) == 0
+		   || !strcmp (base->categories[i], new_categories[i])))
+	{
+	  strcpy (tmp_locale.categories[i], new_categories[i]);
+	  if (i == LC_CTYPE)
+	    {
+	      tmp_locale.wctomb = base->wctomb;
+	      tmp_locale.mbtowc = base->mbtowc;
+	      tmp_locale.cjk_lang = base->cjk_lang;
+	      tmp_locale.ctype_ptr - base->ctype_ptr;
+	    }
+#ifdef __HAVE_LOCALE_INFO__
+	  /* Mark the values as "has still to be copied".  We do this in
+	     two steps to simplify freeing new locale types in case of a
+	     subsequent error. */
+	  tmp_locale.lc_cat[i].ptr = base->lc_cat[i].ptr;
+	  tmp_locale.lc_cat[i].buf = (void *) -1;
+#else
+	  if (i == LC_CTYPE)
+	    strcpy (tmp_locale.ctype_codeset, base->ctype_codeset);
+	  else if (i == LC_MESSAGES)
+	    strcpy (tmp_locale.message_codeset, base->message_codeset);
+#endif
+	}
+      /* Otherwise, if the category is in category_mask, create entry. */
+      else if (((1 << i) & category_mask) != 0)
+	{
+	  /* Nothing to do for "C"/"POSIX" locale. */
+	  if (!strcmp (new_categories[i], "C")
+	      || !strcmp (new_categories[i], "POSIX"))
+	    continue;
 	  /* Otherwise load locale data. */
-	  else if (!__loadlocale (&tmp_locale, i, tmp_locale.categories[i]))
+	  else if (!__loadlocale (&tmp_locale, i, new_categories[i]))
 	    goto error;
 	}
     }
@@ -97,17 +97,21 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
   new_locale = (struct __locale_t *) _calloc_r (p, 1, sizeof *new_locale);
   if (!new_locale)
     goto error;
+  if (base)
+    {
 #ifdef __HAVE_LOCALE_INFO__
-  /* Second step of copying over.  At this point we can safely copy.  Make
+      /* Step 2 of copying over..  Make sure to invalidate the copied buffer
-     sure to invalidate the copied buffer pointers in base, so a subsequent
+	 pointers in base, so the subsequent _freelocale_r (base) doesn't free
-     freelocale (base) doesn't free the buffers now used in the new locale. */
+	 the buffers now used in the new locale. */
-  for (i = 1; i < _LC_LAST; ++i)
+      for (i = 1; i < _LC_LAST; ++i)
-    if (tmp_locale.lc_cat[i].buf == (const void *) -1)
+	if (tmp_locale.lc_cat[i].buf == (const void *) -1)
-      {
+	  {
-	tmp_locale.lc_cat[i].buf = base->lc_cat[i].buf;
+	    tmp_locale.lc_cat[i].buf = base->lc_cat[i].buf;
-	base->lc_cat[i].buf = NULL;
+	    base->lc_cat[i].ptr = base->lc_cat[i].buf = NULL;
-      }
+	  }
 #endif
+      _freelocale_r (p, base);
+    }
 
   *new_locale = tmp_locale;
   return new_locale;
@@ -117,7 +121,8 @@ error:
      Free memory and return NULL.  errno is supposed to be set already. */
 #ifdef __HAVE_LOCALE_INFO__
   for (i = 1; i < _LC_LAST; ++i)
-    if (tmp_locale.lc_cat[i].buf
+    if (((1 << i) & category_mask) != 0
+	&& tmp_locale.lc_cat[i].buf
 	&& tmp_locale.lc_cat[i].buf != (const void *) -1)
       {
 	_free_r (p, (void *) tmp_locale.lc_cat[i].ptr);