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: Wed, 13 Apr 2011 08:31:09 +0200 Message-ID: <87wriybr2q.fsf@rho.meyering.net> 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> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1302676286 24591 80.91.229.12 (13 Apr 2011 06:31:26 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 13 Apr 2011 06:31:26 +0000 (UTC) Cc: Eli Zaretskii , emacs-devel@gnu.org To: Paul Eggert Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Apr 13 08:31:21 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 1Q9tbl-000458-1z for ged-emacs-devel@m.gmane.org; Wed, 13 Apr 2011 08:31:21 +0200 Original-Received: from localhost ([::1]:54766 helo=lists2.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9tbk-0000UP-Av for ged-emacs-devel@m.gmane.org; Wed, 13 Apr 2011 02:31:20 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:52003) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9tbh-0000UH-Of for emacs-devel@gnu.org; Wed, 13 Apr 2011 02:31:18 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q9tbg-0005aD-Op for emacs-devel@gnu.org; Wed, 13 Apr 2011 02:31:17 -0400 Original-Received: from mx.meyering.net ([82.230.74.64]:58949) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9tbc-0005ZC-FD; Wed, 13 Apr 2011 02:31:12 -0400 Original-Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000) id BE8136032C; Wed, 13 Apr 2011 08:31:09 +0200 (CEST) In-Reply-To: <4DA53148.5000903@cs.ucla.edu> (Paul Eggert's message of "Tue, 12 Apr 2011 22:14:48 -0700") Original-Lines: 45 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.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:138453 Archived-At: Paul Eggert wrote: > On 04/12/2011 08:53 AM, Paul Eggert wrote: >> I can do it, in the next day or two. > > OK, I did it, by auditing all uses of emacs_read and emacs_write to > find and fix all negative buffer sizes. However, instead of negative > sizes I found overflows and some other issues, described below; and I > therefore now agree with Eli that emacs_write shouldn't use the same > signature as POSIX 'write'. However, I also agree with Jim that > emacs_write's API need not artificially limit buffer sizes to SSIZE_MAX. > > Here's the key idea of the attached patch. Since > "N = emacs_write (FD, BUF, SIZE)" returns N if successful, and some > value less than N if it fails, the caller can easily determine whether > it succeeded by testing N == SIZE. So on failure there's no need for > emacs_write to return a special value like -1. > > With this in mind, emacs_write can simply return the number of bytes > written, which will always be a size_t value. Jim, this idea is > already used by gnulib's full_write function, so there's good > precedent for departing from the POSIX signature here. And Eli, this > avoids the problem where the size is so large that a signed return > value would overflow and become negative. > > Here are some other bugs in Emacs that I found as part of this audit: > > * Several places in sound.c attempt to stuff a size into an 'int', > which doesn't work. This problem is independent of the above, and > its fix is independent of the above too, so it's split into a > separate patch in the attached bundle. > > * send_process attempts to stuff a size into an 'int' as well. This > can be fixed by changing one of its local variables from int to size_t. > > * emacs_gnutls_write has the same issues as emacs_write, and should > be fixed the same way. > > Attached is a bzr bundle of two proposed patches to fix the problem as > described above. Comments are welcome. That looks like a fine change. Thanks for doing the work. The improved ease of use makes this change worthwhile to me. No more hassling with a mix of signed and unsigned lengths, and thus less risk of misuse and fewer compiler warnings.