From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Jim Meyering Newsgroups: gmane.emacs.devel Subject: Re: oops? read/write vs type of length parameter Date: Mon, 11 Apr 2011 13:08:38 +0200 Message-ID: <87d3ktjb9l.fsf@rho.meyering.net> References: <87wrj1jhfc.fsf@rho.meyering.net> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1302520141 17262 80.91.229.12 (11 Apr 2011 11:09:01 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 11 Apr 2011 11:09:01 +0000 (UTC) Cc: emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Apr 11 13:08:56 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Q9EzH-0004gE-Do for ged-emacs-devel@m.gmane.org; Mon, 11 Apr 2011 13:08:55 +0200 Original-Received: from localhost ([127.0.0.1]:55337 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q9EzG-00008m-4d for ged-emacs-devel@m.gmane.org; Mon, 11 Apr 2011 07:08:54 -0400 Original-Received: from [140.186.70.92] (port=53590 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q9Ez7-000063-Lv for emacs-devel@gnu.org; Mon, 11 Apr 2011 07:08:47 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q9Ez5-0008F7-6P for emacs-devel@gnu.org; Mon, 11 Apr 2011 07:08:44 -0400 Original-Received: from mx.meyering.net ([82.230.74.64]:53839) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9Ez3-0008DS-Hj; Mon, 11 Apr 2011 07:08:41 -0400 Original-Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000) id 83C9D602B9; Mon, 11 Apr 2011 13:08:38 +0200 (CEST) In-Reply-To: (Eli Zaretskii's message of "Mon, 11 Apr 2011 05:44:17 -0400") Original-Lines: 59 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 82.230.74.64 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:138381 Archived-At: Eli Zaretskii wrote: >> From: Jim Meyering >> Date: Mon, 11 Apr 2011 10:55:35 +0200 >> Cc: Eli Zaretskii >> >> In http://bzr.savannah.gnu.org/lh/emacs/trunk/revision/103883 >> you adjusted the signatures of emacs_read and emacs_write. >> >> - extern int emacs_read (int, char *, unsigned int); >> - extern int emacs_write (int, const char *, unsigned int); >> + extern ssize_t emacs_read (int, char *, ssize_t); >> + extern ssize_t emacs_write (int, const char *, ssize_t); > > Yes, that's part of my on-going effort to allow editing of files > larger than 2GB. With that revision, I can finally visit such a large > file, modify it, and save it to disk :-) > >> It's good to see that you corrected the return type to be wider >> (ssize_t) and still signed, just like "int". > > That's what its callers expect: the return value can be positive or > negative. As I said, that part is welcome. It also makes these functions more like read and write. >> However, did you really intend to make the buffer length parameters signed? >> I would have expected those to be of type size_t, not ssize_t. > > We call these functions with an argument of type EMACS_INT, which can > be negative. With read and write, there is already a tension there, since the return value (length or error indicator) is signed, while the length parameter is unsigned, and hence slightly wider. > I don't want it to wind up as a large positive value > inside these functions, I'd rather the functions fail instead. Note > that emacs_write is careful enough to check the sign of that argument, > and if we want a similar guard in emacs_read, we can easily add that. Currently, emacs_write silently ignores an invalid buffer length, treating it just like a length of 0. It'd be better not to ignore such an error. IMHO, an interface that takes a logically unsigned parameter should have an unsigned type. I guess I'm biased towards least-surprise for developers, so I think read- and write-like functions should accept a buffer length argument of type size_t, to be consistent with read and write. To try to protect against bugs by changing API to a signed type may actually cause trouble when callers end up mixing/comparing their newly signed (to accommodate the invented API) and unsigned lengths from standard functions. Another thing to keep in mind: on some older systems, trying to read more than INT_MAX bytes in a single syscall will fail.