Thanks for the comments.
From 99e290cec79754d8d92ec6dcf3c58594782a677b Mon Sep 17 00:00:00 2001
Date: Wed, 5 Apr 2017 17:16:33 -0400
Subject: [PATCH] Check that file argument is a string
* lisp/vc/ediff-diff.el (ediff-exec-process): Check that the argument
passed to `file-local-copy' is a string (Bug#26378). Also fix
the existing comment for this function, and convert it to its
lisp/vc/ediff-diff.el | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/lisp/vc/ediff-diff.el b/lisp/vc/ediff-diff.el
index cfa08ef360..3b2a85afdd 100644
@@ -1134,12 +1134,22 @@ ediff-setup-diff-regions3
-;; Execute PROGRAM asynchronously, unless OS/2, Windows-*, or DOS, or unless
-;; SYNCH is non-nil. BUFFER must be a buffer object, and must be alive. The
-;; OPTIONS arg is a list of options to pass to PROGRAM. It may be a blank
-;; string. All elements in FILES must be strings. We also delete nil from
(defun ediff-exec-process (program buffer synch options &rest files)
+ "Execute the diff PROGRAM.
+The PROGRAM output is sent to BUFFER, which must be a live buffer
+The PROGRAM is executed asynchronously unless `system-type' is
+`windows-nt' or `ms-dos', or unless SYNCH is non-nil.
+OPTIONS is a string of space-separated options to pass to PROGRAM. It
+may be a blank string.
+FILES is a list of filenames to pass to PROGRAM. This list may
+contain a nil element too.
+nil and \"\" elements in OPTIONS and FILES are ignored."
(let ((data (match-data))
;; If this is a buffer job, we are diffing temporary files
;; produced by Emacs with ediff-coding-system-for-write, so
@@ -1151,8 +1161,9 @@ ediff-exec-process
(setq args (append (split-string options)
(mapcar (lambda (file)
- (or (file-local-copy file) file)))
+ (when (stringp file)
+ (or (file-local-copy file) file))))
(setq args (delete "" (delq nil args))) ; delete nil and "" from arguments
;; the --binary option, if present, should be used only for buffer jobs
Now that we're looking at the whole thing, I have a few more comments.
Kaushal Modi <address@hidden> writes:
> + "Execute the diff PROGRAM.
> +The PROGRAM output is sent to BUFFER, which must be a buffer object,
> +and must be alive.
It would flow a bit better to say "which must be a live buffer object".
> +The PROGRAM is executed asynchronously unless the OS is OS/2,
> +Windows-*, or DOS, or unless SYNCH is non-nil.
I don't see any reference to OS/2 in the code, so I think we should just
And I'm not sure what the "-*" in "Windows-*" means;
It means Windows-95, Windows NT, Windows 8, Windows 10, etc.
we should just say "unless `system-type' is `windows-nt' or `ms-dos'".
I am fine with that.
> +OPTIONS is a list of options to pass to PROGRAM. It may be a blank
This seems to be wrong, OPTIONS is not a list. It should say "OPTIONS
is a string of space-separated options to pass to PROGRAM".
You are right; making that change.
> +An element in FILES must be either a string or nil.
> +We also delete nil and \"\" from all arguments."
I think these last 2 sentences should be combined into something like:
"FILES is a list of filenames to pass to PROGRAM; nil and \"\" elements
I have tweaked this part slightly (see patch in this email). Based on the code, nil and "" elements will be removed from a concatenated list of OPTIONS and FILES; not just FILES.