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

[Eli Zaretskii <eliz@gnu.org>]

> Date: Wed, 13 Apr 2011 01:15:01 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: jim@meyering.net, emacs-devel@gnu.org
> 
> On 04/12/2011 11:37 PM, Eli Zaretskii wrote:
> 
> > The current code handles this situation (by looping for what's left
> > to write), while your suggested code will treat that as a fatal error.
> 
> No, the suggested code also loops for what's left to write.
> Perhaps you misread the code? (or am I misunderstanding your comment?)

I was talking about the calls in process.c.  It says now

		    if (XPROCESS (proc)->gnutls_p)
		      rv = emacs_gnutls_write (outfd,
					       XPROCESS (proc),
					       buf, this);
		    else
		      rv = emacs_write (outfd, buf, this);
		      ...
		if (rv < 0)
		  {
		    if (errno == EWOULDBLOCK || errno == EAGAIN)
		      {
		        ...
			rv = 0;
		      }
		    else
		      /* This is a real error.  */
		      report_file_error ("writing to process", Fcons (proc, Qnil));
		  }
		buf += rv;
		len -= rv;
		this -= rv;

and it continues looping if rv is positive.  With your change, it
looks like this:

		    if (XPROCESS (proc)->gnutls_p)
		      written = emacs_gnutls_write (outfd,
					       XPROCESS (proc),
					       buf, this);
		    else
		      written = emacs_write (outfd, buf, this);
		    rv = (written == this ? 0 : -1);
		      ...
		if (rv < 0)
		  {
		    if (errno == EWOULDBLOCK || errno == EAGAIN)
		      {
		        ...
			rv = 0;
		      }
		    else
		      /* This is a real error.  */
		      report_file_error ("writing to process", Fcons (proc, Qnil));
		  }
		buf += rv;
		len -= rv;
		this -= rv;

and it will (with enough luck) exit the loop through
report_file_error, because `rv' becomes negative if `written' is not
equal to `this'.  I was asking whether testing errno for EWOULDBLOCK
and EAGAIN, and the code that deals with when that happens, are good
enough for all the possible reasons that emacs_write and
emacs_gnutls_write could return a partial count of bytes.

> > Emacs cannot have buffers (or any other streams of bytes) that are
> > larger than SSIZE_MAX, because a small number of bits is reserved for
> > the Lisp tags.
> 
> That kind of argument violates abstraction boundaries: sysdep.c
> is supposed to be about system things, and it's not supposed to
> rely on assumptions about Emacs Lisp internals.

sysdep.c is not about system things, it's about Emacs code that
requires platform-dependent techniques.  Most of that indeed deals
with system types, but you will find quite a few Emacs specific
internals there: Lisp_Object and EMACS_TIME data types, calls to
`intern' and Fsleep_for, use of macros such as STRINGP and
FRAME_TERMCAP_P, etc.

> For example, it would be a fairly small change to make Emacs buildable on
> a machine with 32-bit pointers and 64-bit EMACS_INT.

Somehow, I doubt it is a small change.  But if it is, by all means
let's do it now!  What are we waiting for?

>  And this would
> have real advantages: on 32-bit hosts it would remove the arbitrary
> (and really annoying :-) 256 MiB limit on editable files.

Since Emacs 23.2, it's 512 MB, btw, not 256.

> emacs_write should not stand in the way of plausible improvements
> such as this one.

Such a configuration (if it indeed is possible "easily") will need to
revamp several calls to emacs_write anyway, so what type we use there
now is a moot point.

> > It was also about a mistaken
> > call to emacs_write with a negative value in the last argument...
> > 
> > That danger still exists with your proposed version of emacs_write,
> 
> No it doesn't, because I've carefully audited all the Emacs code
> (which is how I found all those *other* bugs :-).  With the
> proposed change, emacs_write is never called with a negative
> argument.

Either you care about future changes or you don't.  If you do, then
auditing just the current callers is not enough, you need to account
for the unknown future (or did you audit that as well?).  If you don't
care about the future, then the assumption that the size of a buffer
or string can never overflow SSIZE_MAX is also based on carefully
auditing the Emacs code, and we can rely on it.