Re: oops? read/write vs type of length parameter

[Paul Eggert]

On looking over that code again, a couple of issues
sprang out.  First, emacs_read should act like emacs_write
with respect to sizes, but the code didn't do that.

Second, no caller should ever pass a negative size value
to either function, and callers should not rely on negative
sizes causing emacs_read and emacs_write to do nothing.
I added a runtime check for this, which I don't think
will ever fail, but I've been surprised in the past.
With that check in place we might as well use size_t for the size,
with the goal of removing the runtime checks once we have
carefully checked that they aren't needed.

Here's the patch I installed for that.

* sysdep.c (emacs_read, emacs_write): Check for negative sizes
since callers should never pass a negative size.
Change the signature to match that of plain 'read' and 'write'; see
<http://lists.gnu.org/archive/html/emacs-devel/2011-04/msg00397.html>.
* lisp.h: Update prototypes of emacs_write and emacs_read.
=== modified file 'src/lisp.h'
--- src/lisp.h  2011-04-10 20:43:08 +0000
+++ src/lisp.h  2011-04-12 08:05:04 +0000
@@ -3346,8 +3346,8 @@
 extern void seed_random (long);
 extern int emacs_open (const char *, int, int);
 extern int emacs_close (int);
-extern ssize_t emacs_read (int, char *, ssize_t);
-extern ssize_t emacs_write (int, const char *, ssize_t);
+extern ssize_t emacs_read (int, char *, size_t);
+extern ssize_t emacs_write (int, const char *, size_t);
 enum { READLINK_BUFSIZE = 1024 };
 extern char *emacs_readlink (const char *, char [READLINK_BUFSIZE]);
 #ifndef HAVE_MEMSET

=== modified file 'src/sysdep.c'
--- src/sysdep.c        2011-04-10 20:43:08 +0000
+++ src/sysdep.c        2011-04-12 08:05:09 +0000
@@ -1826,10 +1826,18 @@
 }

 ssize_t
-emacs_read (int fildes, char *buf, ssize_t nbyte)
+emacs_read (int fildes, char *buf, size_t nbyte)
 {
   register ssize_t rtnval;

+  /* Defend against the possibility that a buggy caller passes a negative NBYTE
+     argument, which would be converted to a large unsigned size_t NBYTE.  This
+     defense prevents callers from doing large writes, unfortunately.  This
+     size restriction can be removed once we have carefully checked that there
+     are no such callers.  */
+  if ((ssize_t) nbyte < 0)
+    abort ();
+
   while ((rtnval = read (fildes, buf, nbyte)) == -1
         && (errno == EINTR))
     QUIT;
@@ -1837,13 +1845,17 @@
 }

 ssize_t
-emacs_write (int fildes, const char *buf, ssize_t nbyte)
+emacs_write (int fildes, const char *buf, size_t nbyte)
 {
   register ssize_t rtnval, bytes_written;

+  /* Defend against negative NBYTE, as in emacs_read.  */
+  if ((ssize_t) nbyte < 0)
+    abort ();
+
   bytes_written = 0;

-  while (nbyte > 0)
+  while (nbyte != 0)
     {
       rtnval = write (fildes, buf, nbyte);