From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#8545: issues with recent doprnt-related changes Date: Wed, 27 Apr 2011 22:34:45 +0300 Message-ID: <83aafb8p4a.fsf@gnu.org> References: <4DB50AB9.6060100@cs.ucla.edu> <83tydmaeo3.fsf@gnu.org> <4DB65FF1.5010003@cs.ucla.edu> Reply-To: Eli Zaretskii NNTP-Posting-Host: lo.gmane.org X-Trace: dough.gmane.org 1303936630 27254 80.91.229.12 (27 Apr 2011 20:37:10 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 27 Apr 2011 20:37:10 +0000 (UTC) Cc: 8545@debbugs.gnu.org To: Paul Eggert Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Apr 27 22:37:03 2011 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1QFBTp-0008JL-Au for geb-bug-gnu-emacs@m.gmane.org; Wed, 27 Apr 2011 22:37:01 +0200 Original-Received: from localhost ([::1]:41454 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFBTo-0005ky-P0 for geb-bug-gnu-emacs@m.gmane.org; Wed, 27 Apr 2011 16:37:00 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:50785) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFBTj-0005eu-OU for bug-gnu-emacs@gnu.org; Wed, 27 Apr 2011 16:36:57 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QFBTi-0000tm-I8 for bug-gnu-emacs@gnu.org; Wed, 27 Apr 2011 16:36:55 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:40833) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFBTi-0000ti-Eo for bug-gnu-emacs@gnu.org; Wed, 27 Apr 2011 16:36:54 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1QFB5e-0000Tc-50; Wed, 27 Apr 2011 16:12:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 27 Apr 2011 20:12:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 8545 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 8545-submit@debbugs.gnu.org id=B8545.13039350751774 (code B ref 8545); Wed, 27 Apr 2011 20:12:02 +0000 Original-Received: (at 8545) by debbugs.gnu.org; 27 Apr 2011 20:11:15 +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 1QFB4s-0000SZ-SL for submit@debbugs.gnu.org; Wed, 27 Apr 2011 16:11:15 -0400 Original-Received: from mtaout20.012.net.il ([80.179.55.166]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QFB4p-0000SK-7f for 8545@debbugs.gnu.org; Wed, 27 Apr 2011 16:11:13 -0400 Original-Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0LKB00400SX0EG00@a-mtaout20.012.net.il> for 8545@debbugs.gnu.org; Wed, 27 Apr 2011 22:34:42 +0300 (IDT) Original-Received: from HOME-C4E4A596F7 ([77.124.32.146]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0LKB002TGT1QV4J0@a-mtaout20.012.net.il>; Wed, 27 Apr 2011 22:34:41 +0300 (IDT) In-reply-to: <4DB65FF1.5010003@cs.ucla.edu> X-012-Sender: halo1@inter.net.il X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Wed, 27 Apr 2011 16:12:02 -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: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:46037 Archived-At: > Date: Mon, 25 Apr 2011 23:02:25 -0700 > From: Paul Eggert > CC: 8545@debbugs.gnu.org > > On 04/25/11 02:00, Eli Zaretskii wrote: > > >> * Format strings never include embedded null bytes, so there's > >> no need for doprnt to support that. > > > > Potentially, someone could call `error' with its first argument taken > > from a Lisp string, which could include null characters. But again, > > this feature was there to begin with, and I see no particular need to > > remove it. > > The feature is buggy, because the code does not check > fmt versus fmt_end every time it increases fmt; it checks > only sometimes. I added more checks, thanks for pointing this out. > "%l" is a strange case anyway, since one cannot reliably use > "%l" as an alias for "%d". For example, the format "%dx" prints > an integer followed by an 'x', but if you try to use "%lx" instead, > it doesn't work. At least, we should remove "%l" as a format > specifier, as it's a rightly-unused feature and it's just asking > for trouble to try to support it. You convinced me, so I removed %l. > >> * If the format string is too long, the alloca inside doprnt will > >> crash Emacs on some hosts. > > > > You are right. I modified doprnt to use SAFE_ALLOCA instead. > > There's no need for alloca or SAFE_ALLOCA or xmalloc or any > dynamic allocator. Instead, convert any width and precision > values to integers, and use "*". For example, if the caller > specifies this: > > "%012345.6789g", 3.14 > > pass this to sprintf: > > "%0*.*g", 12345, 6789, 3.14 I see no reason for such complexity, just to avoid SAFE_ALLOCA. But feel free to make this change, if you think it's important enough. > >> - doprnt uses atoi (&fmtcpy[1]), but surely this isn't right if > >> there are flags such as '-'. > > > > Why not? In that case, atoi will produce a negative value for > > `width', which is already handled by the code. If I'm missing > > something, please point out the specific problems with that. > > I don't see how the negative value is handled correctly. > %-10s means to print a string right-justified, but the code > surely treats it as if it were %0s. ??? %-10s means to print a string LEFT-justified, and the code handles that in this loop (which runs after the string was copied to its destination): if (minlen < 0) { while (minlen < - width && bufsize > 0) { *bufptr++ = ' '; bufsize--; minlen++; } minlen = 0; } I actually tried using %-30s, and it did work correctly (as did %30s). > And other flags > are possible, e.g., atoi will parse "%0-3d" as if the > width were zero, but the width is 3 (the "0" is a flag). The code doesn't call atoi for numeric arguments. It delegates that case to sprintf, which will handle the likes of %0-3d correctly. And for %s and %c the "0" flag is not supported anyway (as stated in the comments) and GCC flags that with a warning. So I see no problem here. > A quick second scan found a minor bug in size parsing: the > expression "n >= SIZE_MAX / 10" should be "n > SIZE_MAX / 10". When they get to messages as long as SIZE_MAX, let them sue me for taking away one byte. verror will reject SIZE_MAX-long messages anyway, so I see no reason to squeeze one more byte here just to throw it away there. > /* Limit the string to sizes that both Emacs and size_t can represent. */ > size_t size_max = min (MOST_POSITIVE_FIXNUM + 1, SIZE_MAX); "MOST_POSITIVE_FIXNUM + 1" is too much, since MOST_POSITIVE_FIXNUM should be able to cover the terminating null character in Emacs. So I used this: size_t size_max = min (MOST_POSITIVE_FIXNUM, SIZE_MAX); > Thanks, can you make a similar change inside doprint? It > also uses xrealloc where xfree+xmalloc would be better. Done. > One other thing, the documentation says that lower-case l > is a flag, but it's a length modifer and not a flag. I fixed the doc on that account.