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: Thu, 28 Apr 2011 01:50:53 -0400 Message-ID: References: <4DB50AB9.6060100@cs.ucla.edu> <83tydmaeo3.fsf@gnu.org> <4DB65FF1.5010003@cs.ucla.edu> <83aafb8p4a.fsf@gnu.org> <4DB8ABEA.3080503@cs.ucla.edu> Reply-To: Eli Zaretskii NNTP-Posting-Host: lo.gmane.org X-Trace: dough.gmane.org 1303970831 25734 80.91.229.12 (28 Apr 2011 06:07:11 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Thu, 28 Apr 2011 06:07:11 +0000 (UTC) Cc: 8545-done@debbugs.gnu.org To: Paul Eggert Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Apr 28 08:07:06 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 1QFKNU-00074R-9b for geb-bug-gnu-emacs@m.gmane.org; Thu, 28 Apr 2011 08:07:04 +0200 Original-Received: from localhost ([::1]:32809 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFKNT-0005Cf-Pj for geb-bug-gnu-emacs@m.gmane.org; Thu, 28 Apr 2011 02:07:03 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:45024) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFKNO-0005CC-9z for bug-gnu-emacs@gnu.org; Thu, 28 Apr 2011 02:07:00 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QFKNM-0007nK-Pi for bug-gnu-emacs@gnu.org; Thu, 28 Apr 2011 02:06:58 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:48523) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFKNM-0007nG-M6 for bug-gnu-emacs@gnu.org; Thu, 28 Apr 2011 02:06:56 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1QFK7z-0005Ke-FF for bug-gnu-emacs@gnu.org; Thu, 28 Apr 2011 01:51:03 -0400 Resent-From: Eli Zaretskii Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: bug-gnu-emacs@gnu.org Resent-Date: Thu, 28 Apr 2011 05:51:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: cc-closed 8545 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Mail-Followup-To: 8545@debbugs.gnu.org, eliz@gnu.org Original-Received: via spool by 8545-done@debbugs.gnu.org id=D8545.130396986220487 (code D ref 8545); Thu, 28 Apr 2011 05:51:03 +0000 Original-Received: (at 8545-done) by debbugs.gnu.org; 28 Apr 2011 05:51:02 +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 1QFK7x-0005KN-0v for submit@debbugs.gnu.org; Thu, 28 Apr 2011 01:51:01 -0400 Original-Received: from fencepost.gnu.org ([140.186.70.10]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QFK7v-0005K9-K0 for 8545-done@debbugs.gnu.org; Thu, 28 Apr 2011 01:51:00 -0400 Original-Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1QFK7p-0000de-I3; Thu, 28 Apr 2011 01:50:53 -0400 In-reply-to: <4DB8ABEA.3080503@cs.ucla.edu> (message from Paul Eggert on Wed, 27 Apr 2011 16:51:06 -0700) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Thu, 28 Apr 2011 01:51:03 -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:46056 Archived-At: > Date: Wed, 27 Apr 2011 16:51:06 -0700 > From: Paul Eggert > CC: 8545@debbugs.gnu.org > > >> 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. > > It's not a question of saving space at run-time. It's a question of > helping the reader. The reader is left wondering: why is that ">=" > there? The reader will be wondering with ">" as well. There's a comment about checking for overflow which should be a good hint, especially since SIZE_MAX is compared against. > And why is there another test "n * 10 > SIZE_MAX - (fmt[1] - > '0')" that always returns 0, no matter what? ??? What happens if n*10 is SIZE_MAX-1 and fmt[1] is '2'? Is the result still zero? > /* Avoid int overflow, because many sprintfs seriously mess up > with widths or precisions greater than INT_MAX. Avoid size_t > overflow, since our counters use size_t. This test is slightly > conservative, for speed and simplicity. */ > if (n >= min (INT_MAX, SIZE_MAX) / 10) > error ("Format width or precision too large"); Sorry, I don't see how this is clearer. The current code after the test is built out of the same building blocks as the test, and therefore the intent and the details of the test are easier to understand than with your variant, which perhaps is mathematically and numerically equivalent, but makes the code reading _harder_ because it severs the syntactical connection between the two. > > "MOST_POSITIVE_FIXNUM + 1" is too much, since MOST_POSITIVE_FIXNUM > > should be able to cover the terminating null character in Emacs. > > Why? Emacs size fields count the bytes in the string, and does not > count the terminating null byte (which is not part of the string). That's not what I know. String positions are zero-based and extend to include the terminating null character. See the relevant parts of the display engine code. > * doprnt invokes strlen to find the length of the format. The > vsnprintf code didn't need to do that: it traversed the format once. > Surely it shouldn't be hard to change doprnt so that it traverses > the format once rather than twice. doprnt is invoked in the context of displaying an error message that throws to top level, and so it doesn't need to be optimized (which will surely make the code more complex and error-prone, and its maintenance harder). > * Sometimes verror will incorrectly truncate a string, even when there > is plenty of memory. verror might call doprnt (buffer, SIZE, m, m + > mlen, ap), and doprnt might discover that a multibyte character is > chopped in half at the end of the output buffer, and might return > (say) SIZE - 2. verror will incorrectly conclude that the output > was just fine, but it wasn't complete. Not an issue, what with the initial buffer size you enlarged to 4000. I needed to artificially lower it to just 2 dozen bytes, just to see the recovery code in action. If someone wants to display a 4001-byte message that ends with a multibyte non-ASCII character, let them be punished for not knowing how to write concisely. > * verror might invoke doprnt two or more times, which means that > doprnt will traverse ap twice. This does not work in general; the C > standard is quite clear that the behavior is undefined in this case. Are there any platforms supported by Emacs where this actually happens? If not, let's forget about this issue until it hits us. I'm closing this bug. We are already well past any real problems, and invested too much energy and efforts of two busy people on this tiny function, all because of your stubborn insistence on using a library function where it doesn't fit the bill. I hope you now have more respect for views and code of others in general, and mine in particular, so we won't need to go through this painful experience again in the future. Let's move on; I still need to work on the bidirectional display of overlay strings and display properties, a job that was already delayed by several precious days due to these disputes and the gratuitous work on the code that should have been left alone in the first place.