Re: [PATCH] Fix newlocale, uselocale, duplocale, is*_l and to*_l

[Jakub Jelinek <jakub at redhat dot com>]

On Sat, Aug 31, 2002 at 11:29:43AM -0700, Roland McGrath wrote:
> D'oh!  I had of course made the newlocale changes but failed to check
> that file in, I don't know how I failed to notice it there in my tree.

I see a few problems with your newlocale.c though
(and I think my version is shorter and had these issues sorted out,
though more eyes see more...).

  if (base == NULL)
    {
      /* Allocate new structure.  */
      result_ptr = (__locale_t) malloc (sizeof (struct __locale_struct));
      if (result_ptr == NULL)
        return NULL;
_nl_remove_locale is not called in this case, leaking locale data.

      /* Install strdup'd names in the new structure's __names array.  */
      for (cnt = 0; cnt < __LC_LAST; ++cnt)
        if (cnt != LC_ALL && result.__names[cnt] != _nl_C_name)
			     ^^^^^^^^^^^^^^^^^^^
As base is NULL, all result.__names[x] are _nl_C_name and nobody changed
it yet, so this loop does nothing. Should be newnames[cnt].
          {
            result.__names[cnt] = __strdup (((category_mask & 1 << cnt) !=  0)
                                            ? newnames[cnt]
                                            : result.__names[cnt]);
This is unnecessary, if cnt is not in category_mark, it will surely be a
_nl_C_name.
            if (result.__names[cnt] == NULL)
              {
                while (cnt-- > 0)
                  if (result.__names[cnt] != _nl_C_name)
                    free ((char *) result.__names[cnt]);
_nl_remove_locale is not called in this case, leaking locale data.
                free (result_ptr);
                return NULL;
              }
          }
    }
  else
    {
      /* We modify the base structure.
         First strdup the names we were given for the new locale.  */

      for (cnt = 0; cnt < __LC_LAST; ++cnt)
        if (cnt != LC_ALL && ((category_mask & 1 << cnt) != 0)
            && result.__names[cnt] != _nl_C_name)
Again, this should probably be newnames[cnt].
          {
            newnames[cnt] = __strdup (newnames[cnt]);
            if (newnames[cnt] == NULL)
              {
                while (cnt-- > 0)
                  if (newnames[cnt] != NULL)
And this should test _nl_C_name instead.
                    free ((char *) newnames[cnt]);
                return NULL;
_nl_remove_locale is not called in this case, leaking locale data.
              }
          }
        else
          newnames[cnt] = NULL;

      /* Now that we can't lose, install the new names.  */
      for (cnt = 0; cnt < __LC_LAST; ++cnt)
        if (newnames[cnt] != NULL)
          {
            if (result.__names[cnt] != _nl_C_name)
              free ((char *) result.__names[cnt]);
            result.__names[cnt] = __strdup (newnames[cnt]);
strdup of strdup?
          }

      result_ptr = base;


	Jakub