From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Philipp Stephani Newsgroups: gmane.emacs.bugs Subject: bug#30005: 27.0.50; call-interactively doesn't work correctly if the interactive specification has an embedded null byte Date: Mon, 22 Jan 2018 22:25:39 +0000 Message-ID: References: <83y3ksspp9.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="94eb2c1abb709c36d4056364e9a0" X-Trace: blaine.gmane.org 1516659888 23198 195.159.176.226 (22 Jan 2018 22:24:48 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 22 Jan 2018 22:24:48 +0000 (UTC) Cc: 30005@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Jan 22 23:24:44 2018 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 1edkVu-0004kz-2X for geb-bug-gnu-emacs@m.gmane.org; Mon, 22 Jan 2018 23:24:26 +0100 Original-Received: from localhost ([::1]:42341 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1edkXu-0002dg-H9 for geb-bug-gnu-emacs@m.gmane.org; Mon, 22 Jan 2018 17:26:30 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60759) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1edkXY-0002Uk-Ax for bug-gnu-emacs@gnu.org; Mon, 22 Jan 2018 17:26:09 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1edkXT-0005pT-AH for bug-gnu-emacs@gnu.org; Mon, 22 Jan 2018 17:26:08 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:59025) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1edkXT-0005pN-66 for bug-gnu-emacs@gnu.org; Mon, 22 Jan 2018 17:26:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1edkXT-0001J1-0G for bug-gnu-emacs@gnu.org; Mon, 22 Jan 2018 17:26:03 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Philipp Stephani Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 22 Jan 2018 22:26:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 30005 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 30005-submit@debbugs.gnu.org id=B30005.15166599594995 (code B ref 30005); Mon, 22 Jan 2018 22:26:02 +0000 Original-Received: (at 30005) by debbugs.gnu.org; 22 Jan 2018 22:25:59 +0000 Original-Received: from localhost ([127.0.0.1]:38687 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1edkXO-0001IV-Gc for submit@debbugs.gnu.org; Mon, 22 Jan 2018 17:25:58 -0500 Original-Received: from mail-lf0-f45.google.com ([209.85.215.45]:34209) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1edkXN-0001IG-3b for 30005@debbugs.gnu.org; Mon, 22 Jan 2018 17:25:57 -0500 Original-Received: by mail-lf0-f45.google.com with SMTP id k19so12593425lfj.1 for <30005@debbugs.gnu.org>; Mon, 22 Jan 2018 14:25:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8HXmW7i9fyCDgaq4lGHq66LkefykLZqON0vj7JS9otw=; b=YdfAnC/FudPM7Ncbd0HiVFI0nsgpSbO6+zlZZxdoFQnSx2ucJktmshg1LSUqfdS7zi uyG6eJiew69Y1ZjRrcZF/eZIJV7JCOVrdUUHocscdunZlHX1lgFRAfsmT4tI6OtT+C94 S62WGyC+gWKhfa/lYz3yhlKW4VSwMHWskwCUJ8ysMrNLXMOJ9ezdKEbzkvadGi4JTLWt BABZoOp8VeQrjZU38LPSiiojR4RPnowL7GdJypuwsoX1UYZQJfmnflFlHVfMY8HhvtLa j7bvIJAUA1GHZFZFWXYNQnlRLt3q5ohMeO4LJ8Ds+rhp/wslmPC9+H4epFi8qFy9SWOp 83RQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8HXmW7i9fyCDgaq4lGHq66LkefykLZqON0vj7JS9otw=; b=R3nLAdszXktsxHo8Hiza0akpjFkFMWV5CuF34Up10DUAl5qXGtPVhcgGDaKPTa/kvo HbNvnv6hPPQmP3YMOrrqZN/TuZ48kaG3309BdNd3ECeohFG7Y42/WhG3wuIejgBsPEev hNDGzS9i1wyxO+S25bv1o62jKRpnYlySyNRb3U+QzkFY7do/K37fwPORNLzGOSSOMmyv nvbkEop6x/OMg7PdWyJaaNa2ROk2vj/VTLON7cag0iG5Bmshx8l4IF1aotExMNgzgMHV lCkyJq90WughV6j0QVLB7ri1Pozg0yR9so5np2I7zs7SsmVQeKL2xV2r0LZbvNJoFoPM b/7Q== X-Gm-Message-State: AKwxytcIWseLkA9qygCu7afTRR0D/wjeEQcBh/lG3HX7wXxewBm3jx/1 BqGUA6UA3tQ9Xc8XOnqfYvsRqTwPEItqNsQh3Sw= X-Google-Smtp-Source: AH8x227SlNDCsRHvGlurItoR7ZskUoaEZRikPcZbMxjT2Ph32pXxVXN061hTgk9d0rvSR4Te9XVUU61ZLAc/NsFA/Ik= X-Received: by 10.46.77.87 with SMTP id a84mr185995ljb.100.1516659950759; Mon, 22 Jan 2018 14:25:50 -0800 (PST) In-Reply-To: <83y3ksspp9.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:142400 Archived-At: --94eb2c1abb709c36d4056364e9a0 Content-Type: text/plain; charset="UTF-8" Eli Zaretskii schrieb am Sa., 20. Jan. 2018 um 13:03 Uhr: > > From: Philipp > > Date: Sat, 06 Jan 2018 12:39:43 +0100 > > > > (call-interactively (lambda (a b) (interactive "sa\0b\ns"))) > > > > the prompt is only "a" and a `wrong-number-of-argument' signal is > > raised. This is because `call-interactively' copies the interactive > > specification to a C string, ignoring embedded nulls. > > No, it copies the spec in its entirety, including embedded null bytes, > but then _processes_ the result as a C string, taking the first null > byte as the end of the string. > > Does the patch below look right, and give good results? > Yes, thanks. Just some minor nits inline to make the code shorter. > > diff --git a/src/callint.c b/src/callint.c > index 2253cdf..3d2ed00 100644 > --- a/src/callint.c > +++ b/src/callint.c > @@ -288,7 +288,8 @@ invoke it. If KEYS is omitted or nil, the return > value of > ptrdiff_t next_event; > > Lisp_Object prefix_arg; > - char *string; > + char *string, *string_end; > + ptrdiff_t string_len; > I think these days (where we require C99) we always declare variables when we first use them. > const char *tem; > > /* If varies[i] > 0, the i'th argument shouldn't just have its value > @@ -396,6 +397,8 @@ invoke it. If KEYS is omitted or nil, the return > value of > /* SPECS is set to a string; use it as an interactive prompt. > Copy it so that STRING will be valid even if a GC relocates SPECS. > */ > SAFE_ALLOCA_STRING (string, specs); > + string_len = SBYTES (specs); > + string_end = string + string_len; > > /* Here if function specifies a string to control parsing the > defaults. */ > > @@ -418,7 +421,7 @@ invoke it. If KEYS is omitted or nil, the return > value of > if (!NILP (record_flag)) > { > char *p = string; > - while (*p) > + while (p < string_end) > { > if (! (*p == 'r' || *p == 'p' || *p == 'P' > || *p == '\n')) > @@ -469,7 +472,7 @@ invoke it. If KEYS is omitted or nil, the return > value of > `funcall-interactively') plus the number of arguments the > interactive spec > would have us give to the function. */ > tem = string; > - for (nargs = 2; *tem; ) > + for (nargs = 2; tem < string_end; ) > { > /* 'r' specifications ("point and mark as 2 numeric args") > produce *two* arguments. */ > @@ -477,7 +480,7 @@ invoke it. If KEYS is omitted or nil, the return > value of > nargs += 2; > else > nargs++; > - tem = strchr (tem, '\n'); > + tem = memchr (tem, '\n', string_len - (tem - string)); > You can write the third argument as string_end - tem. > if (tem) > ++tem; > else > @@ -503,9 +506,12 @@ invoke it. If KEYS is omitted or nil, the return > value of > specbind (Qenable_recursive_minibuffers, Qt); > > tem = string; > - for (i = 2; *tem; i++) > + for (i = 2; tem < string_end; i++) > { > - visargs[1] = make_string (tem + 1, strcspn (tem + 1, "\n")); > + char *pnl = memchr (tem + 1, '\n', string_len - (tem + 1 - string)); > Here you can write the third argument as string_end - (tem + 1). > + ptrdiff_t sz = pnl ? pnl - (tem + 1) : string_end - (tem + 1); > You can write the RHS as (pnl ? pnl : string_end) - (tem + 1). > + > + visargs[1] = make_string (tem + 1, sz); > callint_message = Fformat_message (i - 1, visargs + 1); > > switch (*tem) > @@ -781,7 +787,7 @@ invoke it. If KEYS is omitted or nil, the return > value of > { > /* How many bytes are left unprocessed in the specs string? > (Note that this excludes the trailing null byte.) */ > - ptrdiff_t bytes_left = SBYTES (specs) - (tem - string); > + ptrdiff_t bytes_left = string_len - (tem - string); > Same (string-end - tem). > unsigned letter; > > /* If we have enough bytes left to treat the sequence as a > @@ -803,9 +809,9 @@ invoke it. If KEYS is omitted or nil, the return > value of > if (NILP (visargs[i]) && STRINGP (args[i])) > visargs[i] = args[i]; > > - tem = strchr (tem, '\n'); > + tem = memchr (tem, '\n', string_len - (tem - string)); > again, string_end - tem. > if (tem) tem++; > - else tem = ""; > + else tem = string_end; > } > unbind_to (speccount, Qnil); > > --94eb2c1abb709c36d4056364e9a0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Eli Zaretskii <eli= z@gnu.org> schrieb am Sa., 20. Jan. 2018 um 13:03=C2=A0Uhr:
> From: Phili= pp <p.stephan= i2@gmail.com>
> Date: Sat, 06 Jan 2018 12:39:43 +0100
>
> (call-interactively (lambda (a b) (interactive "sa\0b\ns")))=
>
> the prompt is only "a" and a `wrong-number-of-argument' = signal is
> raised.=C2=A0 This is because `call-interactively' copies the inte= ractive
> specification to a C string, ignoring embedded nulls.

No, it copies the spec in its entirety, including embedded null bytes,
but then _processes_ the result as a C string, taking the first null
byte as the end of the string.

Does the patch below look right, and give good results?

Yes, thanks. Just some minor nits inline to make the code = shorter.
=C2=A0

diff --git a/src/callint.c b/src/callint.c
index 2253cdf..3d2ed00 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -288,7 +288,8 @@ invoke it.=C2=A0 If KEYS is omitted or nil, the return = value of
=C2=A0 =C2=A0ptrdiff_t next_event;

=C2=A0 =C2=A0Lisp_Object prefix_arg;
-=C2=A0 char *string;
+=C2=A0 char *string, *string_end;
+=C2=A0 ptrdiff_t string_len;

I think t= hese days (where we require C99) we always declare variables when we first = use them.
=C2=A0
=C2=A0 =C2=A0const char *tem;

=C2=A0 =C2=A0/* If varies[i] > 0, the i'th argument shouldn't ju= st have its value
@@ -396,6 +397,8 @@ invoke it.=C2=A0 If KEYS is omitted or nil, the return = value of
=C2=A0 =C2=A0/* SPECS is set to a string; use it as an interactive prompt.<= br> =C2=A0 =C2=A0 =C2=A0 Copy it so that STRING will be valid even if a GC relo= cates SPECS.=C2=A0 */
=C2=A0 =C2=A0SAFE_ALLOCA_STRING (string, specs);
+=C2=A0 string_len =3D SBYTES (specs);
+=C2=A0 string_end =3D string + string_len;

=C2=A0 =C2=A0/* Here if function specifies a string to control parsing the = defaults.=C2=A0 */

@@ -418,7 +421,7 @@ invoke it.=C2=A0 If KEYS is omitted or nil, the return = value of
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!NILP (record_flag)) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 char *p =3D = string;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0while (*p) +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0while (p <= ; string_end)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 if (! (*p =3D=3D 'r' || *p =3D=3D 'p' || *p =3D=3D '= ;P'
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|| *p =3D=3D '\n'))
@@ -469,7 +472,7 @@ invoke it.=C2=A0 If KEYS is omitted or nil, the return = value of
=C2=A0 =C2=A0 =C2=A0 `funcall-interactively') plus the number of argume= nts the interactive spec
=C2=A0 =C2=A0 =C2=A0 would have us give to the function.=C2=A0 */
=C2=A0 =C2=A0tem =3D string;
-=C2=A0 for (nargs =3D 2; *tem; )
+=C2=A0 for (nargs =3D 2; tem < string_end; )
=C2=A0 =C2=A0 =C2=A0{
=C2=A0 =C2=A0 =C2=A0 =C2=A0/* 'r' specifications ("point and m= ark as 2 numeric args")
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0produce *two* arguments.=C2=A0 */
@@ -477,7 +480,7 @@ invoke it.=C2=A0 If KEYS is omitted or nil, the return = value of
=C2=A0 =C2=A0 =C2=A0 =C2=A0 nargs +=3D 2;
=C2=A0 =C2=A0 =C2=A0 =C2=A0else
=C2=A0 =C2=A0 =C2=A0 =C2=A0 nargs++;
-=C2=A0 =C2=A0 =C2=A0 tem =3D strchr (tem, '\n');
+=C2=A0 =C2=A0 =C2=A0 tem =3D memchr (tem, '\n', string_len - (tem = - string));

You can write the third arg= ument as string_end - tem.
=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0if (tem)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 ++tem;
=C2=A0 =C2=A0 =C2=A0 =C2=A0else
@@ -503,9 +506,12 @@ invoke it.=C2=A0 If KEYS is omitted or nil, the return= value of
=C2=A0 =C2=A0 =C2=A0specbind (Qenable_recursive_minibuffers, Qt);

=C2=A0 =C2=A0tem =3D string;
-=C2=A0 for (i =3D 2; *tem; i++)
+=C2=A0 for (i =3D 2; tem < string_end; i++)
=C2=A0 =C2=A0 =C2=A0{
-=C2=A0 =C2=A0 =C2=A0 visargs[1] =3D make_string (tem + 1, strcspn (tem + 1= , "\n"));
+=C2=A0 =C2=A0 =C2=A0 char *pnl =3D memchr (tem + 1, '\n', string_l= en - (tem + 1 - string));

Here you can = write the third argument as string_end - (tem=C2=A0+ 1).
=C2=A0
+=C2=A0 =C2=A0 =C2=A0 ptrdiff_t sz =3D pnl ? pnl - (tem + 1) : string_end -= (tem + 1);

You can write the RHS as (p= nl ? pnl : string_end) - (tem=C2=A0+ 1).
=C2=A0
+
+=C2=A0 =C2=A0 =C2=A0 visargs[1] =3D make_string (tem + 1, sz);
=C2=A0 =C2=A0 =C2=A0 =C2=A0callint_message =3D Fformat_message (i - 1, visa= rgs + 1);

=C2=A0 =C2=A0 =C2=A0 =C2=A0switch (*tem)
@@ -781,7 +787,7 @@ invoke it.=C2=A0 If KEYS is omitted or nil, the return = value of
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* How many bytes are left unproc= essed in the specs string?
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(Note that this excl= udes the trailing null byte.)=C2=A0 */
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ptrdiff_t bytes_left =3D SBYTES (= specs) - (tem - string);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ptrdiff_t bytes_left =3D string_l= en - (tem - string);

Same (string-end -= tem).
=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 unsigned letter;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* If we have enough bytes left t= o treat the sequence as a
@@ -803,9 +809,9 @@ invoke it.=C2=A0 If KEYS is omitted or nil, the return = value of
=C2=A0 =C2=A0 =C2=A0 =C2=A0if (NILP (visargs[i]) && STRINGP (args[i= ]))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 visargs[i] =3D args[i];

-=C2=A0 =C2=A0 =C2=A0 tem =3D strchr (tem, '\n');
+=C2=A0 =C2=A0 =C2=A0 tem =3D memchr (tem, '\n', string_len - (tem = - string));

again, string_end - tem.
=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0if (tem) tem++;
-=C2=A0 =C2=A0 =C2=A0 else tem =3D "";
+=C2=A0 =C2=A0 =C2=A0 else tem =3D string_end;
=C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0unbind_to (speccount, Qnil);

--94eb2c1abb709c36d4056364e9a0--