From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: oops? read/write vs type of length parameter Date: Wed, 13 Apr 2011 05:46:27 -0400 Message-ID: References: <87wrj1jhfc.fsf@rho.meyering.net> <87hba5yq0p.fsf@uwakimon.sk.tsukuba.ac.jp> <834o64sxd7.fsf@gnu.org> <4DA3A7F8.1020503@cs.ucla.edu> <83k4f0qijz.fsf@gnu.org> <4DA3DDCD.10700@cs.ucla.edu> <4DA40AFE.8050406@cs.ucla.edu> <4DA47581.9010509@cs.ucla.edu> <4DA53148.5000903@cs.ucla.edu> <4DA55B85.5090305@cs.ucla.edu> Reply-To: Eli Zaretskii NNTP-Posting-Host: lo.gmane.org X-Trace: dough.gmane.org 1302688045 24318 80.91.229.12 (13 Apr 2011 09:47:25 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 13 Apr 2011 09:47:25 +0000 (UTC) Cc: jim@meyering.net, emacs-devel@gnu.org To: Paul Eggert Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Apr 13 11:47:18 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Q9wfO-0001wv-12 for ged-emacs-devel@m.gmane.org; Wed, 13 Apr 2011 11:47:18 +0200 Original-Received: from localhost ([::1]:43594 helo=lists2.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9wfN-000865-2c for ged-emacs-devel@m.gmane.org; Wed, 13 Apr 2011 05:47:17 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:49481) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9weg-0006ql-4V for emacs-devel@gnu.org; Wed, 13 Apr 2011 05:46:40 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q9wea-0000NS-63 for emacs-devel@gnu.org; Wed, 13 Apr 2011 05:46:34 -0400 Original-Received: from fencepost.gnu.org ([140.186.70.10]:53266) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9wea-0000NO-3a for emacs-devel@gnu.org; Wed, 13 Apr 2011 05:46:28 -0400 Original-Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1Q9weZ-0005e3-Us; Wed, 13 Apr 2011 05:46:27 -0400 In-reply-to: <4DA55B85.5090305@cs.ucla.edu> (message from Paul Eggert on Wed, 13 Apr 2011 01:15:01 -0700) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 140.186.70.10 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:138458 Archived-At: > Date: Wed, 13 Apr 2011 01:15:01 -0700 > From: Paul Eggert > 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.