[Bug-gnulib] safe-read.[ch] (safe_read): what do you think?

[Jim Meyering]

I've made the following changes in the coreutils/lib for the upcoming
4.5.4 release:

        * safe-read.c (safe_read): Change type of function
        from ssize_t to size_t.
        * safe-read.h: Update prototype.
        (SAFE_READ_ERROR): Define.

It looks tiny, but IMHO is important, and required careful examination of
each use of safe_read.  There were numerous uses that compared variables
of type size_t and ssize_t (the old return value).  Recent snapshots of
gcc can now warn about some of those, and so I was motivated to fix even
the innocuous ones.  Obviously, if the values concerned are guaranteed
never reach the limit of ssize_t, then there's no real problem, but I
think there was one case where the new warning indicated a potential
problem.  In any case, I think the new interface leads to significantly
cleaner code where safe_read is used.  E.g., there is no need for casts
to avoid warnings.

The question is would any of you object to my putting this change
in gnulib?

Index: safe-read.c
===================================================================
RCS file: /fetish/cu/lib/safe-read.c,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -p -u -r1.14 -r1.15
--- safe-read.c 25 Sep 2002 08:29:53 -0000      1.14
+++ safe-read.c 6 Oct 2002 14:55:01 -0000       1.15
@@ -21,6 +21,7 @@
 #endif
 
 #include <sys/types.h>
+#include <stdlib.h>
 
 #if HAVE_UNISTD_H
 # include <unistd.h>
@@ -34,14 +35,25 @@ extern int errno;
 #include "safe-read.h"
 
 /* Read LEN bytes at PTR from descriptor DESC, retrying if interrupted.
-   Return the actual number of bytes read, zero for EOF, or -1 upon error.  */
+   Return the actual number of bytes read, zero upon EOF,
+   or SAFE_READ_ERROR upon error.
+   Abort if LEN is SAFE_READ_ERROR (aka `(size_t) -1').
+
+   WARNING: although both LEN and the return value are of type size_t,
+   the range of the return value is restricted -- by virtue of being
+   returned from read(2) -- and will never be larger than SSIZE_MAX,
+   with the exception of SAFE_READ_ERROR, of course.
+   So don't test `safe_read (..., N) == N' unless you're sure that
+   N <= SSIZE_MAX.  */
 
-ssize_t
+size_t
 safe_read (int desc, void *ptr, size_t len)
 {
   ssize_t n_chars;
 
-  if (len <= 0)
+  if (len == SAFE_READ_ERROR)
+    abort ();
+  if (len == 0)
     return len;
 
 #ifdef EINTR
Index: safe-read.h
===================================================================
RCS file: /fetish/cu/lib/safe-read.h,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -p -u -r1.3 -r1.4
--- safe-read.h 16 Jul 1998 23:02:19 -0000      1.3
+++ safe-read.h 6 Oct 2002 14:55:12 -0000       1.4
@@ -6,5 +6,7 @@
 # endif
 #endif
 
-ssize_t
+#define SAFE_READ_ERROR ((size_t) -1)
+
+size_t
 safe_read PARAMS ((int desc, void *ptr, size_t len));