Re: c-strtod: handling of out-of-memory

[Jim Meyering]

Bruno Haible <address@hidden> wrote:
> Now, when there is an out-of-memory while c_strtod is called, the handling is
> inconsistent:
>   - If it occurs inside newlocale() or inside strtod_l() or inside strtod(),
>     the function returns 0 with errno set to ENOMEM.
>   - If it occurs inside strdup() of the locale string, the function calls
>     xalloc_die().
>
> Michael Gold said in
> <http://lists.gnu.org/archive/html/bug-gnulib/2009-01/msg00094.html>:
>   "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."
>
> So, to make the function more widely usable and the error handling more
> consistent, I propose to remove the use of xmalloc here, and leave the
> handling of ENOMEM to the caller.
>
> Here is a proposed patch to that effect:
>
> 2009-01-22  Bruno Haible  <address@hidden>
>
>       Make c-strtod, c-strtold usable in libraries.
>       * lib/c-strtod.c: Include string.h instead of xalloc.h.
>       (C_STRTOD): Call strdup instead of xstrdup.
>       * modules/c-strtod (Depends-on): Add strdup-posix, remove xalloc.
>       * modules/c-strtold (Depends-on): Likewise.
>       * doc/c-strtod.texi: Remove the sentence mentioning xalloc_die.
>       * NEWS: Mention the change.
>       Reported by Michael Gold <address@hidden>.

Thanks for working on this.

I've just noticed that now (as of
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=39857ab5806),
*ENDPTR is no longer guaranteed to be set.
That's would be a significant change in contract,
and in fact, it makes at least uptime.c in coreutils
reference the caller's ENDPTR uninitialized, upon
c_strtod failure.

Before, c_strtod had semantics very similar to those of strtod.
The patch below fixes the existing problem.
Once you've adjusted your patch to do the same for the
new return path it adds, you're welcome to commit it.

>From 2606266c5763fb46d7c61d97fc72de02a674c08f Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 23 Jan 2009 16:21:57 +0100
Subject: [PATCH] c-strtod: when ENDPTR is non-NULL, set *ENDPTR in new failure 
path

* lib/c-strtod.c (C_STRTOD) [LC_ALL_MASKC]: Ensure that when
ENDPTR is non-NULL, *ENDPTR is set to NPTR upon failure.
---
 lib/c-strtod.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/lib/c-strtod.c b/lib/c-strtod.c
index 3d0fae9..d2df6b9 100644
--- a/lib/c-strtod.c
+++ b/lib/c-strtod.c
@@ -72,7 +72,11 @@ C_STRTOD (char const *nptr, char **endptr)

   locale_t locale = c_locale ();
   if (!locale)
-    return 0; /* errno is set here */
+    {
+      if (endptr)
+       *endptr = nptr;
+      return 0; /* errno is set here */
+    }

   r = STRTOD_L (nptr, endptr, locale);

--
1.6.1.399.g0d272