* bug#40576: call-process-region does not accept nil as first argument @ 2020-04-12 14:25 Pietro Giorgianni 2020-04-12 16:01 ` Philipp Stephani 0 siblings, 1 reply; 11+ messages in thread From: Pietro Giorgianni @ 2020-04-12 14:25 UTC (permalink / raw) To: 40576 [-- Attachment #1: Type: text/plain, Size: 949 bytes --] Hi, According to the documentation of call-process-region, If START is nil, that means to use the entire buffer contents; END is ignored. But when I run: (call-process-region nil nil "/bin/cat" t (current-buffer)) I get: Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil) call-process-region(nil nil "/bin/cat" t #<buffer *scratch*>) eval((call-process-region nil nil "/bin/cat" t (current-buffer)) nil) elisp--eval-last-sexp(t) eval-last-sexp(t) eval-print-last-sexp(nil) funcall-interactively(eval-print-last-sexp nil) call-interactively(eval-print-last-sexp nil nil) command-execute(eval-print-last-sexp) If, instead, I run: (call-process-region (point-min) (point-max) "/bin/cat" t (current-buffer)) It works. Am I interpreting the documentation wrong? Emacs version: GNU Emacs 26.3 (build 1, x86_64-apple-darwin18.2.0, NS appkit-1671.20 Version 10.14.3 (Build 18D109)) of 2019-09-02 Thank you [-- Attachment #2: Type: text/html, Size: 1324 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 14:25 bug#40576: call-process-region does not accept nil as first argument Pietro Giorgianni @ 2020-04-12 16:01 ` Philipp Stephani 2020-04-12 16:21 ` Eli Zaretskii 2020-04-12 17:07 ` Philipp Stephani 0 siblings, 2 replies; 11+ messages in thread From: Philipp Stephani @ 2020-04-12 16:01 UTC (permalink / raw) To: Pietro Giorgianni; +Cc: 40576 Am So., 12. Apr. 2020 um 17:44 Uhr schrieb Pietro Giorgianni <giorgian@gmail.com>: > > Hi, > > According to the documentation of call-process-region, > If START is nil, that means to use the entire buffer contents; END is > ignored. > > But when I run: > (call-process-region nil nil "/bin/cat" t (current-buffer)) > I get: > Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil) > call-process-region(nil nil "/bin/cat" t #<buffer *scratch*>) > eval((call-process-region nil nil "/bin/cat" t (current-buffer)) nil) > elisp--eval-last-sexp(t) > eval-last-sexp(t) > eval-print-last-sexp(nil) > funcall-interactively(eval-print-last-sexp nil) > call-interactively(eval-print-last-sexp nil nil) > command-execute(eval-print-last-sexp) > > If, instead, I run: > (call-process-region (point-min) (point-max) "/bin/cat" t (current-buffer)) > > It works. > > Am I interpreting the documentation wrong? Nope, looks like a genuine bug (that happens only if DELETE is non-nil). ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 16:01 ` Philipp Stephani @ 2020-04-12 16:21 ` Eli Zaretskii 2020-04-12 16:44 ` Philipp Stephani 2020-04-12 17:07 ` Philipp Stephani 1 sibling, 1 reply; 11+ messages in thread From: Eli Zaretskii @ 2020-04-12 16:21 UTC (permalink / raw) To: Philipp Stephani; +Cc: 40576, giorgian > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Sun, 12 Apr 2020 18:01:40 +0200 > Cc: 40576@debbugs.gnu.org > > > According to the documentation of call-process-region, > > If START is nil, that means to use the entire buffer contents; END is > > ignored. > > > > But when I run: > > (call-process-region nil nil "/bin/cat" t (current-buffer)) > > I get: > > Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil) > > call-process-region(nil nil "/bin/cat" t #<buffer *scratch*>) > > eval((call-process-region nil nil "/bin/cat" t (current-buffer)) nil) > > elisp--eval-last-sexp(t) > > eval-last-sexp(t) > > eval-print-last-sexp(nil) > > funcall-interactively(eval-print-last-sexp nil) > > call-interactively(eval-print-last-sexp nil nil) > > command-execute(eval-print-last-sexp) > > > > If, instead, I run: > > (call-process-region (point-min) (point-max) "/bin/cat" t (current-buffer)) > > > > It works. > > > > Am I interpreting the documentation wrong? > > Nope, looks like a genuine bug (that happens only if DELETE is non-nil). Right. But there's more here than meets the eye, because the change after which we started advertising the special meaning of nil for START exposed a problem: write_region, called from create_temp_file, has special meaning for START = nil: it widens the buffer and writes the entire buffer contents to the temp file. Which isn't right when write_region is called from call-process-region, as it allows access to inaccessible portion of the buffer, something we shouldn't do. So I propose the patch below to fix this bug on the master branch. Any objections? diff --git a/src/callproc.c b/src/callproc.c index 8883415..7f495a3 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -1039,8 +1039,8 @@ DEFUN ("call-process-region", Fcall_process_region, Scall_process_region, START and END are normally buffer positions specifying the part of the buffer to send to the process. -If START is nil, that means to use the entire buffer contents; END is -ignored. +If START is nil, that means to use the entire accessible part of the +buffer; END is ignored. If START is a string, then send that string to the process instead of any buffer contents; END is ignored. The remaining arguments are optional. @@ -1087,6 +1087,14 @@ t (mix it with ordinary output), or a file name string. empty_input = XFIXNUM (start) == XFIXNUM (end); } + if (NILP (start)) + { + XSETFASTINT (start, BEGV); + args[0] = start; + XSETFASTINT (end, ZV); + args[1] = end; + } + if (!empty_input) fd = create_temp_file (nargs, args, &infile); else ^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 16:21 ` Eli Zaretskii @ 2020-04-12 16:44 ` Philipp Stephani 2020-04-12 17:07 ` Eli Zaretskii 2020-04-12 17:07 ` Philipp Stephani 0 siblings, 2 replies; 11+ messages in thread From: Philipp Stephani @ 2020-04-12 16:44 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 40576, Pietro Giorgianni Am So., 12. Apr. 2020 um 18:21 Uhr schrieb Eli Zaretskii <eliz@gnu.org>: > > > From: Philipp Stephani <p.stephani2@gmail.com> > > Date: Sun, 12 Apr 2020 18:01:40 +0200 > > Cc: 40576@debbugs.gnu.org > > > > > According to the documentation of call-process-region, > > > If START is nil, that means to use the entire buffer contents; END is > > > ignored. > > > > > > But when I run: > > > (call-process-region nil nil "/bin/cat" t (current-buffer)) > > > I get: > > > Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil) > > > call-process-region(nil nil "/bin/cat" t #<buffer *scratch*>) > > > eval((call-process-region nil nil "/bin/cat" t (current-buffer)) nil) > > > elisp--eval-last-sexp(t) > > > eval-last-sexp(t) > > > eval-print-last-sexp(nil) > > > funcall-interactively(eval-print-last-sexp nil) > > > call-interactively(eval-print-last-sexp nil nil) > > > command-execute(eval-print-last-sexp) > > > > > > If, instead, I run: > > > (call-process-region (point-min) (point-max) "/bin/cat" t (current-buffer)) > > > > > > It works. > > > > > > Am I interpreting the documentation wrong? > > > > Nope, looks like a genuine bug (that happens only if DELETE is non-nil). > > Right. But there's more here than meets the eye, because the change > after which we started advertising the special meaning of nil for > START exposed a problem: write_region, called from create_temp_file, > has special meaning for START = nil: it widens the buffer and writes > the entire buffer contents to the temp file. Which isn't right when > write_region is called from call-process-region, as it allows access > to inaccessible portion of the buffer, something we shouldn't do. I think that's pretty much intentional. The documentation says "If START is nil, that means to use the entire buffer contents" It specifically doesn't say to only use the accessible portion of the buffer. Given that this behavior probably has been in place since commit 561cb8e159e7eff7a6487a45a1cfab47ba456030 from 1994, it would be rather unwise to introduce such a breaking change. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 16:44 ` Philipp Stephani @ 2020-04-12 17:07 ` Eli Zaretskii 2020-04-12 17:09 ` Philipp Stephani 2020-04-12 17:07 ` Philipp Stephani 1 sibling, 1 reply; 11+ messages in thread From: Eli Zaretskii @ 2020-04-12 17:07 UTC (permalink / raw) To: Philipp Stephani; +Cc: 40576, giorgian > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Sun, 12 Apr 2020 18:44:58 +0200 > Cc: Pietro Giorgianni <giorgian@gmail.com>, 40576@debbugs.gnu.org > > > Right. But there's more here than meets the eye, because the change > > after which we started advertising the special meaning of nil for > > START exposed a problem: write_region, called from create_temp_file, > > has special meaning for START = nil: it widens the buffer and writes > > the entire buffer contents to the temp file. Which isn't right when > > write_region is called from call-process-region, as it allows access > > to inaccessible portion of the buffer, something we shouldn't do. > > I think that's pretty much intentional. The documentation says > > "If START is nil, that means to use the entire buffer contents" That sentence is an addition made in 2016, AFAICT. > It specifically doesn't say to only use the accessible portion of the > buffer. Given that this behavior probably has been in place since > commit 561cb8e159e7eff7a6487a45a1cfab47ba456030 from 1994, it would be > rather unwise to introduce such a breaking change. But then START = nil would work, whereas START = 2 will signal an error if the buffer is narrowed. Does that make sense? ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 17:07 ` Eli Zaretskii @ 2020-04-12 17:09 ` Philipp Stephani 0 siblings, 0 replies; 11+ messages in thread From: Philipp Stephani @ 2020-04-12 17:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 40576, Pietro Giorgianni Am So., 12. Apr. 2020 um 19:07 Uhr schrieb Eli Zaretskii <eliz@gnu.org>: > > > From: Philipp Stephani <p.stephani2@gmail.com> > > Date: Sun, 12 Apr 2020 18:44:58 +0200 > > Cc: Pietro Giorgianni <giorgian@gmail.com>, 40576@debbugs.gnu.org > > > > > Right. But there's more here than meets the eye, because the change > > > after which we started advertising the special meaning of nil for > > > START exposed a problem: write_region, called from create_temp_file, > > > has special meaning for START = nil: it widens the buffer and writes > > > the entire buffer contents to the temp file. Which isn't right when > > > write_region is called from call-process-region, as it allows access > > > to inaccessible portion of the buffer, something we shouldn't do. > > > > I think that's pretty much intentional. The documentation says > > > > "If START is nil, that means to use the entire buffer contents" > > That sentence is an addition made in 2016, AFAICT. Sure, but the behavior has been around for so long that it's very likely somebody already relies on it. > > > It specifically doesn't say to only use the accessible portion of the > > buffer. Given that this behavior probably has been in place since > > commit 561cb8e159e7eff7a6487a45a1cfab47ba456030 from 1994, it would be > > rather unwise to introduce such a breaking change. > > But then START = nil would work, whereas START = 2 will signal an > error if the buffer is narrowed. Does that make sense? No, absolutely not. But I think it's too late to change it. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 16:44 ` Philipp Stephani 2020-04-12 17:07 ` Eli Zaretskii @ 2020-04-12 17:07 ` Philipp Stephani 2020-04-12 17:24 ` Eli Zaretskii 1 sibling, 1 reply; 11+ messages in thread From: Philipp Stephani @ 2020-04-12 17:07 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 40576, Pietro Giorgianni Am So., 12. Apr. 2020 um 18:44 Uhr schrieb Philipp Stephani <p.stephani2@gmail.com>: > > Am So., 12. Apr. 2020 um 18:21 Uhr schrieb Eli Zaretskii <eliz@gnu.org>: > > > > > From: Philipp Stephani <p.stephani2@gmail.com> > > > Date: Sun, 12 Apr 2020 18:01:40 +0200 > > > Cc: 40576@debbugs.gnu.org > > > > > > > According to the documentation of call-process-region, > > > > If START is nil, that means to use the entire buffer contents; END is > > > > ignored. > > > > > > > > But when I run: > > > > (call-process-region nil nil "/bin/cat" t (current-buffer)) > > > > I get: > > > > Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil) > > > > call-process-region(nil nil "/bin/cat" t #<buffer *scratch*>) > > > > eval((call-process-region nil nil "/bin/cat" t (current-buffer)) nil) > > > > elisp--eval-last-sexp(t) > > > > eval-last-sexp(t) > > > > eval-print-last-sexp(nil) > > > > funcall-interactively(eval-print-last-sexp nil) > > > > call-interactively(eval-print-last-sexp nil nil) > > > > command-execute(eval-print-last-sexp) > > > > > > > > If, instead, I run: > > > > (call-process-region (point-min) (point-max) "/bin/cat" t (current-buffer)) > > > > > > > > It works. > > > > > > > > Am I interpreting the documentation wrong? > > > > > > Nope, looks like a genuine bug (that happens only if DELETE is non-nil). > > > > Right. But there's more here than meets the eye, because the change > > after which we started advertising the special meaning of nil for > > START exposed a problem: write_region, called from create_temp_file, > > has special meaning for START = nil: it widens the buffer and writes > > the entire buffer contents to the temp file. Which isn't right when > > write_region is called from call-process-region, as it allows access > > to inaccessible portion of the buffer, something we shouldn't do. > > I think that's pretty much intentional. The documentation says > > "If START is nil, that means to use the entire buffer contents" > > It specifically doesn't say to only use the accessible portion of the > buffer. Given that this behavior probably has been in place since > commit 561cb8e159e7eff7a6487a45a1cfab47ba456030 from 1994, it would be > rather unwise to introduce such a breaking change. While the behavior of call-process-region and write-region is unfortunate in this respect, I think it's way too late to change it now. We should rather explicitly call out in the docstrings that buffer restrictions are ignored. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 17:07 ` Philipp Stephani @ 2020-04-12 17:24 ` Eli Zaretskii 2020-04-12 17:37 ` Philipp Stephani 0 siblings, 1 reply; 11+ messages in thread From: Eli Zaretskii @ 2020-04-12 17:24 UTC (permalink / raw) To: Philipp Stephani; +Cc: 40576, giorgian > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Sun, 12 Apr 2020 19:07:59 +0200 > Cc: Pietro Giorgianni <giorgian@gmail.com>, 40576@debbugs.gnu.org > > We should rather explicitly call out in the docstrings that buffer > restrictions are ignored. But the restrictions aren't ignored. They are only ignored if START is nil. That makes no sense. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 17:24 ` Eli Zaretskii @ 2020-04-12 17:37 ` Philipp Stephani 2020-04-12 18:29 ` Eli Zaretskii 0 siblings, 1 reply; 11+ messages in thread From: Philipp Stephani @ 2020-04-12 17:37 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 40576, Pietro Giorgianni Am So., 12. Apr. 2020 um 19:25 Uhr schrieb Eli Zaretskii <eliz@gnu.org>: > > > From: Philipp Stephani <p.stephani2@gmail.com> > > Date: Sun, 12 Apr 2020 19:07:59 +0200 > > Cc: Pietro Giorgianni <giorgian@gmail.com>, 40576@debbugs.gnu.org > > > > We should rather explicitly call out in the docstrings that buffer > > restrictions are ignored. > > But the restrictions aren't ignored. They are only ignored if START > is nil. That makes no sense. I completely agree that it doesn't make sense. I'm just saying that the behavior has been there for so long that we realistically can't change it any more. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 17:37 ` Philipp Stephani @ 2020-04-12 18:29 ` Eli Zaretskii 0 siblings, 0 replies; 11+ messages in thread From: Eli Zaretskii @ 2020-04-12 18:29 UTC (permalink / raw) To: Philipp Stephani; +Cc: 40576, giorgian > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Sun, 12 Apr 2020 19:37:07 +0200 > Cc: Pietro Giorgianni <giorgian@gmail.com>, 40576@debbugs.gnu.org > > > But the restrictions aren't ignored. They are only ignored if START > > is nil. That makes no sense. > > I completely agree that it doesn't make sense. I'm just saying that > the behavior has been there for so long that we realistically can't > change it any more. I think we can change it, but I won't argue further. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#40576: call-process-region does not accept nil as first argument 2020-04-12 16:01 ` Philipp Stephani 2020-04-12 16:21 ` Eli Zaretskii @ 2020-04-12 17:07 ` Philipp Stephani 1 sibling, 0 replies; 11+ messages in thread From: Philipp Stephani @ 2020-04-12 17:07 UTC (permalink / raw) To: Pietro Giorgianni; +Cc: 40576-done Am So., 12. Apr. 2020 um 18:01 Uhr schrieb Philipp Stephani <p.stephani2@gmail.com>: > > Am So., 12. Apr. 2020 um 17:44 Uhr schrieb Pietro Giorgianni > <giorgian@gmail.com>: > > > > Hi, > > > > According to the documentation of call-process-region, > > If START is nil, that means to use the entire buffer contents; END is > > ignored. > > > > But when I run: > > (call-process-region nil nil "/bin/cat" t (current-buffer)) > > I get: > > Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil) > > call-process-region(nil nil "/bin/cat" t #<buffer *scratch*>) > > eval((call-process-region nil nil "/bin/cat" t (current-buffer)) nil) > > elisp--eval-last-sexp(t) > > eval-last-sexp(t) > > eval-print-last-sexp(nil) > > funcall-interactively(eval-print-last-sexp nil) > > call-interactively(eval-print-last-sexp nil nil) > > command-execute(eval-print-last-sexp) > > > > If, instead, I run: > > (call-process-region (point-min) (point-max) "/bin/cat" t (current-buffer)) > > > > It works. > > > > Am I interpreting the documentation wrong? > > Nope, looks like a genuine bug (that happens only if DELETE is non-nil). I've now fixed this on master (commit 42306747d8). ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-04-12 18:29 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-12 14:25 bug#40576: call-process-region does not accept nil as first argument Pietro Giorgianni 2020-04-12 16:01 ` Philipp Stephani 2020-04-12 16:21 ` Eli Zaretskii 2020-04-12 16:44 ` Philipp Stephani 2020-04-12 17:07 ` Eli Zaretskii 2020-04-12 17:09 ` Philipp Stephani 2020-04-12 17:07 ` Philipp Stephani 2020-04-12 17:24 ` Eli Zaretskii 2020-04-12 17:37 ` Philipp Stephani 2020-04-12 18:29 ` Eli Zaretskii 2020-04-12 17:07 ` Philipp Stephani
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).