From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#24206: 25.1; Curly quotes generate invalid strings, leading to a segfault Date: Tue, 16 Aug 2016 14:07:05 -0700 Organization: UCLA Computer Science Department Message-ID: <4822bfeb-c507-a9ff-93bc-1d27ba93b9d7@cs.ucla.edu> References: <8337m7h1dp.fsf@gnu.org> <83zioffew5.fsf@gnu.org> <83popaf1yz.fsf@gnu.org> <87bn0u3rqc.fsf@linux-m68k.org> <83mvkdg91i.fsf@gnu.org> <8b78f23f-4a4f-e568-b760-3350ca7bb8d3@cs.ucla.edu> <83d1l8g3zs.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------293763337082D1FF79ACA8AC" X-Trace: blaine.gmane.org 1471382013 800 195.159.176.226 (16 Aug 2016 21:13:33 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 16 Aug 2016 21:13:33 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 Cc: p.stephani2@gmail.com, johnw@gnu.org, schwab@linux-m68k.org, 24206@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Aug 16 23:13:29 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bZlfs-0007or-Jb for geb-bug-gnu-emacs@m.gmane.org; Tue, 16 Aug 2016 23:13:28 +0200 Original-Received: from localhost ([::1]:44144 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZlas-0008JS-BD for geb-bug-gnu-emacs@m.gmane.org; Tue, 16 Aug 2016 17:08:18 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:58330) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZlai-0008H9-JH for bug-gnu-emacs@gnu.org; Tue, 16 Aug 2016 17:08:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bZlac-00058v-Ff for bug-gnu-emacs@gnu.org; Tue, 16 Aug 2016 17:08:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:33592) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZlac-00058p-Cd for bug-gnu-emacs@gnu.org; Tue, 16 Aug 2016 17:08:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bZlac-0002Qf-6h for bug-gnu-emacs@gnu.org; Tue, 16 Aug 2016 17:08:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 16 Aug 2016 21:08:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24206 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 24206-submit@debbugs.gnu.org id=B24206.14713816389278 (code B ref 24206); Tue, 16 Aug 2016 21:08:02 +0000 Original-Received: (at 24206) by debbugs.gnu.org; 16 Aug 2016 21:07:18 +0000 Original-Received: from localhost ([127.0.0.1]:59535 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bZlZu-0002Pa-ED for submit@debbugs.gnu.org; Tue, 16 Aug 2016 17:07:18 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:58865) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bZlZs-0002PE-9Q for 24206@debbugs.gnu.org; Tue, 16 Aug 2016 17:07:16 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 8EC18161165; Tue, 16 Aug 2016 14:07:09 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id ymJSgNXid21K; Tue, 16 Aug 2016 14:07:08 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 9687B161308; Tue, 16 Aug 2016 14:07:08 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id qcIDBu5g979h; Tue, 16 Aug 2016 14:07:08 -0700 (PDT) Original-Received: from [192.168.1.9] (unknown [100.32.155.148]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 6E7D3161165; Tue, 16 Aug 2016 14:07:08 -0700 (PDT) In-Reply-To: <83d1l8g3zs.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.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" Xref: news.gmane.org gmane.emacs.bugs:122291 Archived-At: This is a multi-part message in MIME format. --------------293763337082D1FF79ACA8AC Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Eli Zaretskii wrote: > I've reviewed the changes you pushed to master for fixing this bug, > and I must say that most of them look like purely stylistic changes, > to make the code more to your personal liking. The actual bugs were > very few and minor, and didn't necessitate such thorough changes. The changes were motivated by bug fixes, not style. Although the bugs wer= e=20 mostly minor (e.g., generating bogus NUL bytes due to miscounting) it's f= ine to=20 fix minor bugs. I did change nearby style (indenting as per GNU style, sw= itching=20 some locals to C99-style decl-after-statement, etc.) but none of the chan= ges=20 were pervasive or were intended for the emacs-25 branch, and it's fine to= make=20 such changes in master. One of the bugs was O(N**2) performance when reallocating a temporary buf= fer.=20 While I was at it, I changed the code to allocate a small temp buffer on = the=20 stack to avoid a malloc/free in the usual case, which should be a small w= in.=20 This accounts for many changes that a quick glance might give the mistake= n=20 impression of being stylistic. > without any comments as to why we handle the input string as > multibyte for the rest of the function, I think this will confuse > someone down the road. OK, I added some comments along those lines (see attached patch). > I think the refactoring already introduced a > regression. This comment appears to be about changes made in May for Bug#23425, not a= bout=20 code changes I made recently. The May changes were not a regression; they= were=20 intended and are documented in etc/NEWS. Alan Mackenzie felt strongly tha= t some=20 changes were needed in this area. See commit=20 433d366dc7b053048abf710d790ff62421dd1570. > maybe we should just go to the original code, after fixing the > immediate bugs, as I proposed in a patch yesterday? No, the code in master should be uniformly better than what's in emacs-25= . It would be nice to have good tests for substitute-command-keys, of cours= e. (We=20 can all add this to our lists of things to do. :-) --------------293763337082D1FF79ACA8AC Content-Type: text/x-diff; name="0001-Omit-substitute-command-keys-code-no-longer-needed.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0001-Omit-substitute-command-keys-code-no-longer-needed.patc"; filename*1="h" =46rom bc5a55262382da9cc4b1999df7ce195b7f825d4b Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 16 Aug 2016 13:16:49 -0700 Subject: [PATCH] Omit substitute-command-keys code no longer needed * src/doc.c (Fsubstitute_command_keys): Remove duplicate initializations. --- src/doc.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/doc.c b/src/doc.c index 37a731b..4b91831 100644 --- a/src/doc.c +++ b/src/doc.c @@ -743,10 +743,12 @@ Otherwise, return a new string. */) if (NILP (string)) return Qnil; =20 + /* If STRING contains non-ASCII unibyte data, process its + properly-encoded multibyte equivalent instead. This simplifies + the implementation and is OK since substitute-command-keys is + intended for use only on text strings. Keep STRING around, since + it will be returned if no changes occur. */ Lisp_Object str =3D Fstring_make_multibyte (string); - tem =3D Qnil; - keymap =3D Qnil; - name =3D Qnil; =20 enum text_quoting_style quoting_style =3D text_quoting_style (); =20 @@ -905,6 +907,8 @@ Otherwise, return a new string. */) } =20 subst_string: + /* Convert non-ASCII unibyte data to properly-encoded multibyte, + for the same reason STRING was converted to STR. */ tem =3D Fstring_make_multibyte (tem); start =3D SDATA (tem); length =3D SCHARS (tem); --=20 2.7.4 --------------293763337082D1FF79ACA8AC--