From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#8435: misuse of error ("...%d...", ...) on 64-bit hosts Date: Fri, 08 Apr 2011 16:34:12 -0700 Organization: UCLA Computer Science Department Message-ID: <4D9F9B74.6050908@cs.ucla.edu> References: <4D9CC60D.2090301@cs.ucla.edu> <4D9D68D8.6060200@cs.ucla.edu> <8339ltvrok.fsf@gnu.org> <4D9E21FB.70802@cs.ucla.edu> <83vcypt8zf.fsf@gnu.org> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1302305853 12936 80.91.229.12 (8 Apr 2011 23:37:33 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 8 Apr 2011 23:37:33 +0000 (UTC) Cc: 8435@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Apr 09 01:37:29 2011 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1Q8LF1-0004HX-SG for geb-bug-gnu-emacs@m.gmane.org; Sat, 09 Apr 2011 01:37:28 +0200 Original-Received: from localhost ([127.0.0.1]:39154 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q8LF1-0007FO-DC for geb-bug-gnu-emacs@m.gmane.org; Fri, 08 Apr 2011 19:37:27 -0400 Original-Received: from [140.186.70.92] (port=51408 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q8LEt-0007EA-Rd for bug-gnu-emacs@gnu.org; Fri, 08 Apr 2011 19:37:20 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q8LEs-0000YV-NZ for bug-gnu-emacs@gnu.org; Fri, 08 Apr 2011 19:37:19 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:56155) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q8LEs-0000YQ-Lw for bug-gnu-emacs@gnu.org; Fri, 08 Apr 2011 19:37:18 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1Q8LCf-0007tE-W0; Fri, 08 Apr 2011 19:35:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 08 Apr 2011 23:35:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 8435 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 8435-submit@debbugs.gnu.org id=B8435.130230566230280 (code B ref 8435); Fri, 08 Apr 2011 23:35:01 +0000 Original-Received: (at 8435) by debbugs.gnu.org; 8 Apr 2011 23:34:22 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q8LC2-0007sL-Dm for submit@debbugs.gnu.org; Fri, 08 Apr 2011 19:34:22 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q8LBz-0007s5-2k for 8435@debbugs.gnu.org; Fri, 08 Apr 2011 19:34:20 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 4920D39E8083; Fri, 8 Apr 2011 16:34:13 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id N9WJIKSBFjMJ; Fri, 8 Apr 2011 16:34:12 -0700 (PDT) Original-Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 4A38D39E8082; Fri, 8 Apr 2011 16:34:12 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.15) Gecko/20110307 Fedora/3.1.9-0.39.b3pre.fc14 Thunderbird/3.1.9 In-Reply-To: <83vcypt8zf.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Fri, 08 Apr 2011 19:35:01 -0400 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:45701 Archived-At: On 04/08/2011 01:58 AM, Eli Zaretskii wrote: > So I think it would be better to fix these problems as follows: > ... > . Fix doprnt to avoid overflow when EMACS_INT is a 64-bit type, if > it could overflow. (I don't see such a danger, but maybe I > overlook something.) That wouldn't work, not because doprnt overflows with EMACS_INT, but because doprnt doesn't work with ordinary 'int': it treats all integer arguments as if they were EMACS_INT, and this relies on unportable va_arg behavior. It's true that doprnt also has overflow problems on 64-bit hosts. For example, it overruns a buffer when formatting a string whose length doesn't fit in 'int'. But that's a separate issue. No doubt these problems could be worked around with sufficient hacking, but why bother? The main reason doprnt exists is that vsnprintf didn't exist back when doprnt was written, so we had to write it ourselves. But now that we can rely on vsnprintf, let's use it rather than continuing to maintain our reinvented buggy wheel. > From now on, any code that needs to use %c for displaying a > character codepoint will need to convert it manually before calling > the message functions. Yes, that's true. It's not a problem now, since there's only one occurrence of this situation in the Emacs source code, and it is easy to treat it as a one-off. If it turns into a problem, we can easily address it, by replacing this sort of code: int codepoint = (whatever); error ("Invalid FINAL-CHAR %c, it should be `0'..`~'", codepoint); with something like this: int codepoint = (whatever); error ("Invalid FINAL-CHAR %s, it should be `0'..`~'", cvt (codepoint)); where 'cvt' converts a codepoint to a string suitable for 'error'. > I don't think we have any reason to support strings longer than > INT_MAX in these functions. They are used to display messages in the > echo area/minibuffer, so they can hardly be close to INT_MAX anyway. > We could simply document that and move on. For bullet-proof code, we > could even check the length and truncate the string before passing it > to verror or its subroutines. OK, thanks, then here's a further patch to do something along those lines. It reliably reports "memory full" when the resulting error string is longer than INT_MAX; that's better than crashing, which is what doprnt currently does. * eval.c: Port to Windows vsnprintf (Bug#8435). Include . (SIZE_MAX): Define if the headers do not. (verror): Do not give up if vsnprintf returns a negative count. Instead, grow the buffer. This ports to Windows vsnprintf, which does not conform to C99. Problem reported by Eli Zaretskii. Also, simplify the allocation scheme, by avoiding the need for calling realloc, and removing the ALLOCATED variable. === modified file 'src/eval.c' --- src/eval.c 2011-04-07 05:19:50 +0000 +++ src/eval.c 2011-04-08 23:08:55 +0000 @@ -18,6 +18,7 @@ #include +#include #include #include "lisp.h" #include "blockinput.h" @@ -30,6 +31,10 @@ #include "xterm.h" #endif +#ifndef SIZE_MAX +# define SIZE_MAX ((size_t) -1) +#endif + /* This definition is duplicated in alloc.c and keyboard.c. */ /* Putting it in lisp.h makes cc bomb out! */ @@ -1978,36 +1983,37 @@ { char buf[4000]; size_t size = sizeof buf; - size_t size_max = (size_t) -1; + size_t size_max = + min (MOST_POSITIVE_FIXNUM, min (INT_MAX, SIZE_MAX - 1)) + 1; char *buffer = buf; - int allocated = 0; int used; Lisp_Object string; while (1) { used = vsnprintf (buffer, size, m, ap); + if (used < 0) - used = 0; - if (used < size) + { + /* Non-C99 vsnprintf, such as w32, returns -1 when SIZE is too small. + Guess a larger USED to work around the incompatibility. */ + used = (size <= size_max / 2 ? 2 * size + : size < size_max ? size_max - 1 + : size_max); + } + else if (used < size) break; - if (size <= size_max / 2) - size *= 2; - else if (size < size_max) - size = size_max; - else + if (size_max <= used) memory_full (); - if (allocated) - buffer = (char *) xrealloc (buffer, size); - else - { - buffer = (char *) xmalloc (size); - allocated = 1; - } + size = used + 1; + + if (buffer != buf) + xfree (buffer); + buffer = (char *) xmalloc (size); } string = make_string (buffer, used); - if (allocated) + if (buffer != buf) xfree (buffer); xsignal1 (Qerror, string);