c-strtod threading and documentation problems

[Michael Gold]

Hi,

I was planning to use the c-strtod module in a library, but noticed some
problems when looking at its implementation.

On platforms without strtod_l, it saves the current locale, switches to
"C" using setlocale, calls strtod, and restores the old locale. This is
not threadsafe because setlocale affects all threads in a program (and
also because setlocale doesn't seem to be threadsafe on all platforms).
The attached program demonstrates the problem -- if some thread calls
strtod while another is calling c_strtod, the call to strtod may return
an incorrect result in locales where the decimal point isn't '.'.

c_strtod will also call abort() on these platforms if there's no memory
to copy the old locale string, which makes it inappropriate for use in
libraries. This should be documented in the function description.

Finally, the documentation states c_strtod "operates as if the locale
encoding was ASCII", but the code doesn't actually convert the argument
from ASCII to the execution character set.

The first problem could probably be fixed by calling localeconv and
replacing '.' with the proper decimal point, which may require a
temporary memory allocation. Maybe another strtod function should be
defined that can return ENOMEM -- c_strtod could call this, and abort if
it gets that error (actually, c_strtod should be renamed xc_strtod, but
this would break compatibility with existing apps that use it).

-- Michael

#include <locale.h>
#include <stdio.h>
#include <string.h>
#include <pthread.h>
#include <errno.h>
#include <stdlib.h>
#include <math.h>

/* Run this program with LC_ALL=fr_FR.utf-8 (or some other locale with
 * "," as the decimal point) to demonstrate that c_strtod will affect
 * other threads on platforms without strtod_l.
 *
 * Compile with the -pthread flag if using GCC.
 */

double c_strtod (char const *nptr, char **endptr)
{
  double r;
  char *saved_locale = setlocale (LC_NUMERIC, NULL);

  if (saved_locale)
    {
      saved_locale = strdup (saved_locale);
      if (!saved_locale) abort();
      setlocale (LC_NUMERIC, "C");
    }

  r = strtod(nptr, endptr);

  if (saved_locale)
    {
      setlocale (LC_NUMERIC, saved_locale);
      free (saved_locale);
    }

  return r;
}

void *strtod_thread(void *arg)
{
  int i, use_c_strtod = *((int*)arg);
  for (i=0; ; ++i)
    {
      char *str = use_c_strtod ? "1234.567" : "1234,567";
      double val = (use_c_strtod ? c_strtod : strtod)(str, NULL);

      if (!(fabs(val - 1234.567) < 0.1))
        printf("fail val=%f use_c_strtod=%d i=%d\n",
                val, use_c_strtod, i);
    }
}

int main(int argc, char **argv)
{
  pthread_t th[2];
  int i, arg[2] = {0, 1};

  setlocale(LC_ALL, "");
  for (i = 0; i < 2; ++i)
    {
      if (pthread_create(&th[i], NULL, strtod_thread, &arg[i]))
        printf("thread creation failed, i=%d\n", i);
    }
  pthread_join(th[0], NULL);
  return 0;
}

signature.asc
Description: Digital signature