Re: trimming getloadavg's dependencies

[Paul Eggert]

On 02/07/2011 02:46 AM, Bruno Haible wrote:
> it's certainly worth avoiding all the
> avalanche of dependencies :-)

Oh my yes!

I hacked on it a bit more and came up with two patches,
one for getloadavg itself, and one for stdlib (which
getloadavg depends on).  The getloadavg patch is similar
to what I already sent in (and reflects your comments).
The stdlib patch removes an unnecessary dependency on stdint.

With these two patches, Emacs needs only four new
gnulib files to use gnulib getloadavg, namely
lib/getloadavg.c, lib/stdlib.in.h, m4/getloadavg.m4,
and m4/stdlib_h.m4.

I have tested this with Emacs on RHEL 5.5 and Solaris 8,
and with coreutils on RHEL 5.5.  None of them need
to build getloadavg, though getloadavg.o builds anyway.

Comments are welcome.  I'll also circulate the Emacs
changes on emacs-devel.

======================================================================

---
 ChangeLog          |   18 +++++++++++++++
 lib/getloadavg.c   |   59 +++++++++++++++++++++++++++++++++++++---------------
 modules/getloadavg |    3 --
 3 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c1ba821..647ac3b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2011-02-07  Paul Eggert  <address@hidden>
+
+       getloadavg: don't depend on c-strtod, cloexec, fcntl-safer
+       See the thread rooted at
+       <http://lists.gnu.org/archive/html/bug-gnulib/2011-02/msg00090.html>.
+       * lib/getloadavg.c: Do not include c-strtod.h, cloexec.h, or fcntl--.h.
+       Include <fcntl.h> only if (defined __linux__ || defined __CYGWIN__
+       || defined SUNOS_5 || (defined LOAD_AVE_TYPE && !  defined
+       __VMS)); previously it was always included (via fcntl--.h).
+       (getloadavg): Do not use c_strtod.  Instead, approximate it by
+       hand; this is good enough for load averages.  Also, do not use
+       set_cloexec_flag; instead, use the O_CLOEXEC and F_DUPFD_CLOEXEC
+       flags directly if available and don't bother otherwise.  (Packages
+       that need the extra reliability should use the modules that define
+       these flags on older platforms that lack them.)
+       * modules/getloadavg (Depends-on): Remove c-strtod, cloexec,
+       fcntl-safer.
+
 2011-02-06  Bruno Haible  <address@hidden>
 
        Rename module 'wctype' to 'wctype-h'.
diff --git a/lib/getloadavg.c b/lib/getloadavg.c
index 18a5960..96a6aa4 100644
--- a/lib/getloadavg.c
+++ b/lib/getloadavg.c
@@ -108,8 +108,6 @@
 #  include <sys/param.h>
 # endif
 
-# include "c-strtod.h"
-# include "cloexec.h"
 # include "intprops.h"
 
 /* The existing Emacs configuration files define a macro called
@@ -372,7 +370,6 @@
 #    endif /* NLIST_STRUCT */
 
 #    ifdef SUNOS_5
-#     include <fcntl.h>
 #     include <kvm.h>
 #     include <kstat.h>
 #    endif
@@ -461,7 +458,10 @@
 #  include <sys/dg_sys_info.h>
 # endif
 
-# include "fcntl--.h"
+# if (defined __linux__ || defined __CYGWIN__ || defined SUNOS_5        \
+      || (defined LOAD_AVE_TYPE && ! defined __VMS))
+#  include <fcntl.h>
+# endif
 
 /* Avoid static vars inside a function since in HPUX they dump as pure.  */
 
@@ -618,19 +618,30 @@ getloadavg (double loadavg[], int nelem)
 
   for (elem = 0; elem < nelem; elem++)
     {
-      char *endptr;
-      double d;
+      double numerator = 0;
+      double denominator = 1;
+      bool have_digit = false;
+
+      while (*ptr == ' ')
+        ptr++;
 
-      errno = 0;
-      d = c_strtod (ptr, &endptr);
-      if (ptr == endptr || (d == 0 && errno != 0))
+      /* Finish if this number is missing, and report an error if all
+         were missing.  */
+      if (! ('0' <= *ptr && *ptr <= '9'))
         {
           if (elem == 0)
             return -1;
           break;
         }
-      loadavg[elem] = d;
-      ptr = endptr;
+
+      while ('0' <= *ptr && *ptr <= '9')
+        numerator = 10 * numerator + (*ptr++ - '0');
+
+      if (*ptr == '.')
+        for (ptr++; '0' <= *ptr && *ptr <= '9'; ptr++)
+          numerator = 10 * numerator + (*ptr - '0'), denominator *= 10;
+
+      loadavg[elem++] = numerator / denominator;
     }
 
   return elem;
@@ -940,13 +951,27 @@ getloadavg (double loadavg[], int nelem)
   if (!getloadavg_initialized)
     {
 #  ifndef SUNOS_5
-      channel = open ("/dev/kmem", O_RDONLY);
-      if (channel >= 0)
+      /* Set the channel to close on exec, so it does not
+         litter any child's descriptor table.  */
+#   ifndef O_CLOEXEC
+#    define O_CLOEXEC 0
+#   endif
+      int fd = open ("/dev/kmem", O_RDONLY | O_CLOEXEC);
+      if (0 <= fd)
         {
-          /* Set the channel to close on exec, so it does not
-             litter any child's descriptor table.  */
-          set_cloexec_flag (channel, true);
-          getloadavg_initialized = true;
+#   if F_DUPFD_CLOEXEC
+          if (fd <= STDERR_FILENO)
+            {
+              int fd1 = fcntl (fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
+              close (fd);
+              fd = fd1;
+            }
+#   endif
+          if (0 <= fd)
+            {
+              channel = fd;
+              getloadavg_initialized = true;
+            }
         }
 #  else /* SUNOS_5 */
       /* We pass 0 for the kernel, corefile, and swapfile names
diff --git a/modules/getloadavg b/modules/getloadavg
index 7865b8f..fcbce4f 100644
--- a/modules/getloadavg
+++ b/modules/getloadavg
@@ -6,10 +6,7 @@ lib/getloadavg.c
 m4/getloadavg.m4
 
 Depends-on:
-c-strtod
-cloexec
 extensions
-fcntl-safer
 intprops
 stdbool
 stdlib
-- 
1.7.4

======================================================================

---
 ChangeLog       |   12 ++++++++++++
 lib/stdlib.in.h |    3 +--
 m4/random_r.m4  |   11 ++++++++++-
 m4/stdlib_h.m4  |    9 +--------
 modules/stdlib  |    1 -
 5 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 647ac3b..f735325 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 2011-02-07  Paul Eggert  <address@hidden>
 
+       stdlib: don't depend on stdint
+       * lib/stdlib.in.h: Don't include <stdint.h> merely because
+       GNULIB_POSIXCHECK is defined.  GNULIB_POSIXCHECK seems to
+       be independent of whether stdint.h is needed.
+       * m4/random_r.m4 (gl_FUNC_RANDOM_R): Check for struct random_data
+       here, instead of ...
+       * m4/stdlib_h.m4 (gl_STDLIB_H): ... here.  Applications that need
+       struct random_data should be using the random_r module, not just
+       the stdlib module (which wouldn't make sense: what package needs
+       just struct random_data without also needing random_r?).
+       * modules/stdlib (Depends-on): Remove stdint.
+
        getloadavg: don't depend on c-strtod, cloexec, fcntl-safer
        See the thread rooted at
        <http://lists.gnu.org/archive/html/bug-gnulib/2011-02/msg00090.html>.
diff --git a/lib/stdlib.in.h b/lib/stdlib.in.h
index a9b284c..b3bb9d4 100644
--- a/lib/stdlib.in.h
+++ b/lib/stdlib.in.h
@@ -55,8 +55,7 @@
 # include <random.h>
 #endif
 
-#if address@hidden@ || (@GNULIB_RANDOM_R@ && address@hidden@) \
-    || defined GNULIB_POSIXCHECK
+#if address@hidden@ || (@GNULIB_RANDOM_R@ && address@hidden@)
 # include <stdint.h>
 #endif
 
diff --git a/m4/random_r.m4 b/m4/random_r.m4
index 8dd24f0..81ede1f 100644
--- a/m4/random_r.m4
+++ b/m4/random_r.m4
@@ -1,4 +1,4 @@
-# serial 1
+# serial 2
 dnl Copyright (C) 2008-2011 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -7,6 +7,15 @@ dnl with or without modifications, as long as this notice is 
preserved.
 AC_DEFUN([gl_FUNC_RANDOM_R],
 [
   AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
+
+  AC_CHECK_TYPES([struct random_data],
+    [], [HAVE_STRUCT_RANDOM_DATA=0],
+    [[#include <stdlib.h>
+      #if HAVE_RANDOM_H
+      # include <random.h>
+      #endif
+    ]])
+
   AC_CHECK_FUNCS([random_r])
   if test $ac_cv_func_random_r = no; then
     HAVE_RANDOM_R=0
diff --git a/m4/stdlib_h.m4 b/m4/stdlib_h.m4
index a71468f..28eb1a0 100644
--- a/m4/stdlib_h.m4
+++ b/m4/stdlib_h.m4
@@ -1,4 +1,4 @@
-# stdlib_h.m4 serial 34
+# stdlib_h.m4 serial 35
 dnl Copyright (C) 2007-2011 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -15,13 +15,6 @@ AC_DEFUN([gl_STDLIB_H],
     HAVE_RANDOM_H=0
   fi
   AC_SUBST([HAVE_RANDOM_H])
-  AC_CHECK_TYPES([struct random_data],
-    [], [HAVE_STRUCT_RANDOM_DATA=0],
-    [[#include <stdlib.h>
-      #if HAVE_RANDOM_H
-      # include <random.h>
-      #endif
-    ]])
 
   dnl Check for declarations of anything we want to poison if the
   dnl corresponding gnulib module is not in use, and which is not
diff --git a/modules/stdlib b/modules/stdlib
index 273095e..a45f4a4 100644
--- a/modules/stdlib
+++ b/modules/stdlib
@@ -10,7 +10,6 @@ arg-nonnull
 c++defs
 include_next
 stddef
-stdint
 unistd
 warn-on-use
 
-- 
1.7.4