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#8545: issues with recent doprnt-related changes Date: Sun, 24 Apr 2011 22:46:33 -0700 Organization: UCLA Computer Science Department Message-ID: <4DB50AB9.6060100@cs.ucla.edu> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1303711663 24484 80.91.229.12 (25 Apr 2011 06:07:43 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 25 Apr 2011 06:07:43 +0000 (UTC) To: 8545@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Apr 25 08:07:38 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 1QEExM-000814-TT for geb-bug-gnu-emacs@m.gmane.org; Mon, 25 Apr 2011 08:07:37 +0200 Original-Received: from localhost ([::1]:33453 helo=lists2.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEExM-0004ac-Ao for geb-bug-gnu-emacs@m.gmane.org; Mon, 25 Apr 2011 02:07:36 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:50591) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEExJ-0004aT-PT for bug-gnu-emacs@gnu.org; Mon, 25 Apr 2011 02:07:34 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QEExI-0003tb-Qr for bug-gnu-emacs@gnu.org; Mon, 25 Apr 2011 02:07:33 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:37855) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEExI-0003tW-PS for bug-gnu-emacs@gnu.org; Mon, 25 Apr 2011 02:07:32 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1QEEeP-0004Qd-VR; Mon, 25 Apr 2011 01:48:01 -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: Mon, 25 Apr 2011 05:48:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 8545 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.130371042216949 (code B ref -1); Mon, 25 Apr 2011 05:48:01 +0000 Original-Received: (at submit) by debbugs.gnu.org; 25 Apr 2011 05:47: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 1QEEdR-0004PJ-QA for submit@debbugs.gnu.org; Mon, 25 Apr 2011 01:47:02 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QEEdP-0004Op-SU for submit@debbugs.gnu.org; Mon, 25 Apr 2011 01:47:00 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QEEdJ-0000yv-RZ for submit@debbugs.gnu.org; Mon, 25 Apr 2011 01:46:54 -0400 Original-Received: from lists.gnu.org ([140.186.70.17]:42980) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEEdJ-0000yq-Q4 for submit@debbugs.gnu.org; Mon, 25 Apr 2011 01:46:53 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:34848) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEEdI-0003LV-Ua for bug-gnu-emacs@gnu.org; Mon, 25 Apr 2011 01:46:53 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QEEdH-0000yZ-Oi for bug-gnu-emacs@gnu.org; Mon, 25 Apr 2011 01:46:52 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:57012) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEEdH-0000xq-Fb; Mon, 25 Apr 2011 01:46:51 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id A617B39E80E0; Sun, 24 Apr 2011 22:46:42 -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 vHaWohnIK7vt; Sun, 24 Apr 2011 22:46:42 -0700 (PDT) Original-Received: from [192.168.1.10] (pool-71-189-109-235.lsanca.fios.verizon.net [71.189.109.235]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 19A6639E8082; Sun, 24 Apr 2011 22:46:42 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Thunderbird/3.1.8 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Mon, 25 Apr 2011 01:48: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: , 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:45980 Archived-At: This is a followup to Bug#8435. Eli invited me to review the recent doprnt-related changes, so here's a quick review: * doprnt returns size_t. But Stefan wrote that he prefers sizes to be signed values, and doprnt always returns a value that can fit in EMACS_INT. So shouldn't doprnt return EMACS_INT, as it did before? * doprnt supports only a small subset of the standard printf formats, but this subset is not documented. It's unclear what the subset is. Or it's a superset of the subset, with %S and %l? Anyway, this should be documented clearly in the lead comment. * I suggest that every feature in doprnt be a feature that is actually needed and used; this will simplify maintainance. * Format strings never include embedded null bytes, so there's no need for doprnt to support that. * If the format string is too long, the alloca inside doprnt will crash Emacs on some hosts. I suggest removing the alloca, instituting a fixed size limit on format specifiers, and documenting that limit. Since user code cannot ever supply one of these formats, that should be good enough. * The width features of doprnt (e.g., %25s) are never used. That part of the code is still buggy; please see some comments below. I suggest removing it entirely; this will simplify things. But if not: - doprnt mishandles format specifications such as %0.0.0d. It passes them off to printf, and this results in undefined behavior, near as I can tell. - doprnt uses atoi (&fmtcpy[1]), but surely this isn't right if there are flags such as '-'. - Quite possibly there are other problems in this area, but I didn't want to spend further time reviewing a never-used feature. * In this code, in verror: used = doprnt (buffer, size, m, m + mlen, ap); /* Note: the -1 below is because `doprnt' returns the number of bytes excluding the terminating null byte, and it always terminates with a null byte, even when producing a truncated message. */ if (used < size - 1) break; I don't see the reason for the "- 1". If you replace this with: used = doprnt (buffer, size, m, m + mlen, ap); if (used < size) break; the code should still work, because, when used < size, the buffer should be properly null-terminated. If it isn't then there's something wrong with doprnt, no? * In this code, in verror: else if (size < size_max - 1) size = size_max - 1; there's no need for the "- 1"s. Just use this: else if (size < size_max) size = size_max; * This code in verror: if (buffer == buf) buffer = (char *) xmalloc (size); else buffer = (char *) xrealloc (buffer, size); uses xrealloc, which is unnecessarily expensive, as it may copy the buffer's contents even though they are entirely garbage here. Use this instead, to avoid the useless copy: if (buffer != buf) xfree (buffer); buffer = (char *) xmalloc (size);