* bug#7112: 24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
@ 2010-09-27 0:21 Drew Adams
2011-07-13 13:18 ` Lars Magne Ingebrigtsen
0 siblings, 1 reply; 6+ messages in thread
From: Drew Adams @ 2010-09-27 0:21 UTC (permalink / raw)
To: 7112
You can call `dired' passing a cons arg that includes a list of file
names. An empty file name causes this ugly error:
(error "Args out of range: \"\", -1")
The reason is the following code near the end of
`ls-lisp-insert-directory':
;; If not full-directory-p, FILE *must not* end in /, as
;; file-attributes will not recognize a symlink to a directory,
;; so must make it a relative filename as ls does:
(if (file-name-absolute-p file) (setq file (expand-file-name file)))
(if (eq (aref file (1- (length file))) ?/)
(setq file (substring file 0 -1)))
`ls-lisp-insert-directory' should in fact do nothing at all (no-op) if
FILE is "". The entire body should be wrapped in this:
(when (> (length file) 0)
...)
In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
of 2010-09-20 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4) --no-opt --cflags
-Ic:/imagesupport/include'
^ permalink raw reply [flat|nested] 6+ messages in thread* bug#7112: 24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
2010-09-27 0:21 bug#7112: 24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE Drew Adams
@ 2011-07-13 13:18 ` Lars Magne Ingebrigtsen
2011-07-13 15:13 ` Drew Adams
0 siblings, 1 reply; 6+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-07-13 13:18 UTC (permalink / raw)
To: Drew Adams; +Cc: 7112
"Drew Adams" <drew.adams@oracle.com> writes:
> You can call `dired' passing a cons arg that includes a list of file
> names. An empty file name causes this ugly error:
>
> (error "Args out of range: \"\", -1")
>
> The reason is the following code near the end of
> `ls-lisp-insert-directory':
I think this should most likely be fixed in the caller (i.e., in
dired.el). What's the backtrace for this bug?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog http://lars.ingebrigtsen.no/
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#7112: 24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
2011-07-13 13:18 ` Lars Magne Ingebrigtsen
@ 2011-07-13 15:13 ` Drew Adams
2011-08-02 20:06 ` Lars Magne Ingebrigtsen
0 siblings, 1 reply; 6+ messages in thread
From: Drew Adams @ 2011-07-13 15:13 UTC (permalink / raw)
To: 'Lars Magne Ingebrigtsen'; +Cc: 7112
> > You can call `dired' passing a cons arg that includes a list of file
> > names. An empty file name causes this ugly error:
> > (error "Args out of range: \"\", -1")
> > The reason is the following code near the end of
> > `ls-lisp-insert-directory':
>
> I think this should most likely be fixed in the caller (i.e., in
> dired.el).
What does that mean? dired.el is a file, not a caller.
> What's the backtrace for this bug?
Why not try it yourself, using the emacs -Q recipe:
M-: (dired '("foobar" "111.el" ""))
Anyway:
Debugger entered--Lisp error: (args-out-of-range "" -1)
aref("" -1)
(eq (aref file (1- (length file))) 47)
(if (eq (aref file (1- (length file))) 47) (setq file (substring file 0 -1)))
(if (or wildcard-regexp full-directory-p) (let* ((dir (file-name-as-directory
file)) (default-directory dir) (file-alist (directory-files-and-attributes dir
nil wildcard-regexp t (if (memq 110 switches) (quote integer) (quote string))))
(sum 0) (max-uid-len 0) (max-gid-len 0) (max-file-size 0) total-line files elt
short file-size attr fuid fgid uid-len gid-len) (cond ((memq 65 switches) (setq
file-alist (ls-lisp-delete-matching "^\\.\\.?$" file-alist))) ((not (memq 97
switches)) (setq file-alist (ls-lisp-delete-matching "^\\." file-alist)))) (setq
file-alist (ls-lisp-handle-switches file-alist switches)) (if (memq 67 switches)
(ls-lisp-column-format file-alist) (setq total-line (cons (point) (car-safe
file-alist))) (dolist (elt file-alist) (setq attr (cdr elt) fuid (nth 2 attr)
uid-len (if (stringp fuid) (string-width fuid) (length (format "%d" fuid))) fgid
(nth 3 attr) gid-len (if (stringp fgid) (string-width fgid) (length (format "%d"
fgid))) file-size (nth 7 attr)) (if (> uid-len max-uid-len) (setq max-uid-len
uid-len)) (if (> gid-len max-gid-len) (setq max-gid-len gid-len)) (if (>
file-size max-file-size) (setq max-file-size file-size))) (setq
ls-lisp-uid-d-fmt (format " %%-%dd" max-uid-len)) (setq ls-lisp-uid-s-fmt
(format " %%-%ds" max-uid-len)) (setq ls-lisp-gid-d-fmt (format " %%-%dd"
max-gid-len)) (setq ls-lisp-gid-s-fmt (format " %%-%ds" max-gid-len)) (setq
ls-lisp-filesize-d-fmt (format " %%%dd" (if (memq 115 switches) (length (format
"%.0f" ...)) (length (format "%.0f" max-file-size))))) (setq
ls-lisp-filesize-f-fmt (format " %%%d.0f" (if (memq 115 switches) (length
(format "%.0f" ...)) (length (format "%.0f" max-file-size))))) (setq files
file-alist) (while files (setq elt (car files) files (cdr files) short (car elt)
attr (cdr elt) file-size (nth 7 attr)) (and attr (setq sum (+ file-size (if ...
sum ...))) (insert (ls-lisp-format short attr file-size switches time-index))))
(save-excursion (goto-char (car total-line)) (or (cdr total-line) (insert "(No
match)\n")) (insert (format "total %.0f\n" (fceiling (/ sum 1024.0)))))) (if
(memq 82 switches) (while file-alist (setq elt (car file-alist) file-alist (cdr
file-alist)) (when (and (eq (cadr elt) t) (not (string-match "\\`\\.\\.?/?\\'"
...))) (setq elt (expand-file-name (car elt) dir)) (insert "\n" elt ":\n")
(ls-lisp-insert-directory elt switches time-index wildcard-regexp
full-directory-p))))) (if (file-name-absolute-p file) (setq file
(expand-file-name file))) (if (eq (aref file (1- (length file))) 47) (setq file
(substring file 0 -1))) (let ((fattr (file-attributes file (quote string)))) (if
fattr (insert (ls-lisp-format (if (memq 70 switches) (ls-lisp-classify-file file
fattr) file) fattr (nth 7 fattr) switches time-index)) (message "%s: doesn't
exist or is inaccessible" file) (ding) (sit-for 2))))
ls-lisp-insert-directory("" (97 108) nil nil nil)
(condition-case err (ls-lisp-insert-directory file switches
(ls-lisp-time-index switches) wildcard-regexp full-directory-p) (invalid-regexp
(if (equal (cadr err) "Unmatched [ or [^") (progn (setq wildcard-regexp (if
(memq 66 switches) "[^~]\\'") file (file-relative-name orig-file))
(ls-lisp-insert-directory file switches (ls-lisp-time-index switches) nil
full-directory-p)) (signal (car err) (cdr err)))))
(if handler (funcall handler (quote insert-directory) file switches wildcard
full-directory-p) (if (string-match "--dired " switches) (setq switches
(replace-match "" nil nil switches))) (setq switches (delete 32 (delete 45
(append switches nil)))) (if (and ls-lisp-support-shell-wildcards (string-match
"[[?*]" file) (not (file-exists-p file))) (progn (or (not (eq (aref file (1-
...)) 47)) (setq file (substring file 0 (1- (length file))))) (setq wildcard
t))) (if wildcard (setq wildcard-regexp (if ls-lisp-support-shell-wildcards
(wildcard-to-regexp (file-name-nondirectory file)) (file-name-nondirectory
file)) file (file-name-directory file)) (if (memq 66 switches) (setq
wildcard-regexp "[^~]\\'"))) (condition-case err (ls-lisp-insert-directory file
switches (ls-lisp-time-index switches) wildcard-regexp full-directory-p)
(invalid-regexp (if (equal (cadr err) "Unmatched [ or [^") (progn (setq
wildcard-regexp (if (memq 66 switches) "[^~]\\'") file (file-relative-name
orig-file)) (ls-lisp-insert-directory file switches (ls-lisp-time-index
switches) nil full-directory-p)) (signal (car err) (cdr err))))) (save-excursion
(goto-char (point-min)) (when (re-search-forward "^total" nil t) (let
((available (get-free-disk-space "."))) (when available (replace-match "total
used in directory") (end-of-line) (insert " available " available))))))
(let ((handler (find-file-name-handler (expand-file-name file) (quote
insert-directory))) (orig-file file) wildcard-regexp) (if handler (funcall
handler (quote insert-directory) file switches wildcard full-directory-p) (if
(string-match "--dired " switches) (setq switches (replace-match "" nil nil
switches))) (setq switches (delete 32 (delete 45 (append switches nil)))) (if
(and ls-lisp-support-shell-wildcards (string-match "[[?*]" file) (not
(file-exists-p file))) (progn (or (not (eq (aref file ...) 47)) (setq file
(substring file 0 (1- ...)))) (setq wildcard t))) (if wildcard (setq
wildcard-regexp (if ls-lisp-support-shell-wildcards (wildcard-to-regexp
(file-name-nondirectory file)) (file-name-nondirectory file)) file
(file-name-directory file)) (if (memq 66 switches) (setq wildcard-regexp
"[^~]\\'"))) (condition-case err (ls-lisp-insert-directory file switches
(ls-lisp-time-index switches) wildcard-regexp full-directory-p) (invalid-regexp
(if (equal (cadr err) "Unmatched [ or [^") (progn (setq wildcard-regexp (if ...
"[^~]\\'") file (file-relative-name orig-file)) (ls-lisp-insert-directory file
switches (ls-lisp-time-index switches) nil full-directory-p)) (signal (car err)
(cdr err))))) (save-excursion (goto-char (point-min)) (when (re-search-forward
"^total" nil t) (let ((available (get-free-disk-space "."))) (when available
(replace-match "total used in directory") (end-of-line) (insert " available "
available)))))))
(if ls-lisp-use-insert-directory-program (funcall original-insert-directory
file switches wildcard full-directory-p) (let ((handler (find-file-name-handler
(expand-file-name file) (quote insert-directory))) (orig-file file)
wildcard-regexp) (if handler (funcall handler (quote insert-directory) file
switches wildcard full-directory-p) (if (string-match "--dired " switches) (setq
switches (replace-match "" nil nil switches))) (setq switches (delete 32 (delete
45 (append switches nil)))) (if (and ls-lisp-support-shell-wildcards
(string-match "[[?*]" file) (not (file-exists-p file))) (progn (or (not (eq ...
47)) (setq file (substring file 0 ...))) (setq wildcard t))) (if wildcard (setq
wildcard-regexp (if ls-lisp-support-shell-wildcards (wildcard-to-regexp
(file-name-nondirectory file)) (file-name-nondirectory file)) file
(file-name-directory file)) (if (memq 66 switches) (setq wildcard-regexp
"[^~]\\'"))) (condition-case err (ls-lisp-insert-directory file switches
(ls-lisp-time-index switches) wildcard-regexp full-directory-p) (invalid-regexp
(if (equal (cadr err) "Unmatched [ or [^") (progn (setq wildcard-regexp ... file
...) (ls-lisp-insert-directory file switches ... nil full-directory-p)) (signal
(car err) (cdr err))))) (save-excursion (goto-char (point-min)) (when
(re-search-forward "^total" nil t) (let ((available ...)) (when available
(replace-match "total used in directory") (end-of-line) (insert " available "
available))))))))
insert-directory("" "-al" nil nil)
apply(insert-directory ("" "-al" nil nil))
(let* ((inhibit-file-name-handlers (\` (tramp-completion-file-name-handler
cygwin-mount-name-hook-function cygwin-mount-map-drive-hook-function \, (and (eq
inhibit-file-name-operation operation) inhibit-file-name-handlers))))
(inhibit-file-name-operation operation)) (apply operation args))
tramp-completion-run-real-handler(insert-directory ("" "-al" nil nil))
(if (and fn tramp-mode (or (eq tramp-syntax (quote sep)) (featurep (quote
tramp)) (and (boundp (quote partial-completion-mode)) (symbol-value (quote
partial-completion-mode))) (featurep (quote ido)) (featurep (quote icicles))))
(save-match-data (apply (cdr fn) args)) (tramp-completion-run-real-handler
operation args))
(let ((directory-sep-char 47) (fn (assoc operation
tramp-completion-file-name-handler-alist))) (if (and fn tramp-mode (or (eq
tramp-syntax (quote sep)) (featurep (quote tramp)) (and (boundp (quote
partial-completion-mode)) (symbol-value (quote partial-completion-mode)))
(featurep (quote ido)) (featurep (quote icicles)))) (save-match-data (apply (cdr
fn) args)) (tramp-completion-run-real-handler operation args)))
tramp-completion-file-name-handler(insert-directory "" "-al" nil nil)
funcall(tramp-completion-file-name-handler insert-directory "" "-al" nil nil)
(if handler (funcall handler (quote insert-directory) file switches wildcard
full-directory-p) (if (string-match "--dired " switches) (setq switches
(replace-match "" nil nil switches))) (setq switches (delete 32 (delete 45
(append switches nil)))) (if (and ls-lisp-support-shell-wildcards (string-match
"[[?*]" file) (not (file-exists-p file))) (progn (or (not (eq (aref file (1-
...)) 47)) (setq file (substring file 0 (1- (length file))))) (setq wildcard
t))) (if wildcard (setq wildcard-regexp (if ls-lisp-support-shell-wildcards
(wildcard-to-regexp (file-name-nondirectory file)) (file-name-nondirectory
file)) file (file-name-directory file)) (if (memq 66 switches) (setq
wildcard-regexp "[^~]\\'"))) (condition-case err (ls-lisp-insert-directory file
switches (ls-lisp-time-index switches) wildcard-regexp full-directory-p)
(invalid-regexp (if (equal (cadr err) "Unmatched [ or [^") (progn (setq
wildcard-regexp (if (memq 66 switches) "[^~]\\'") file (file-relative-name
orig-file)) (ls-lisp-insert-directory file switches (ls-lisp-time-index
switches) nil full-directory-p)) (signal (car err) (cdr err))))) (save-excursion
(goto-char (point-min)) (when (re-search-forward "^total" nil t) (let
((available (get-free-disk-space "."))) (when available (replace-match "total
used in directory") (end-of-line) (insert " available " available))))))
(let ((handler (find-file-name-handler (expand-file-name file) (quote
insert-directory))) (orig-file file) wildcard-regexp) (if handler (funcall
handler (quote insert-directory) file switches wildcard full-directory-p) (if
(string-match "--dired " switches) (setq switches (replace-match "" nil nil
switches))) (setq switches (delete 32 (delete 45 (append switches nil)))) (if
(and ls-lisp-support-shell-wildcards (string-match "[[?*]" file) (not
(file-exists-p file))) (progn (or (not (eq (aref file ...) 47)) (setq file
(substring file 0 (1- ...)))) (setq wildcard t))) (if wildcard (setq
wildcard-regexp (if ls-lisp-support-shell-wildcards (wildcard-to-regexp
(file-name-nondirectory file)) (file-name-nondirectory file)) file
(file-name-directory file)) (if (memq 66 switches) (setq wildcard-regexp
"[^~]\\'"))) (condition-case err (ls-lisp-insert-directory file switches
(ls-lisp-time-index switches) wildcard-regexp full-directory-p) (invalid-regexp
(if (equal (cadr err) "Unmatched [ or [^") (progn (setq wildcard-regexp (if ...
"[^~]\\'") file (file-relative-name orig-file)) (ls-lisp-insert-directory file
switches (ls-lisp-time-index switches) nil full-directory-p)) (signal (car err)
(cdr err))))) (save-excursion (goto-char (point-min)) (when (re-search-forward
"^total" nil t) (let ((available (get-free-disk-space "."))) (when available
(replace-match "total used in directory") (end-of-line) (insert " available "
available)))))))
(if ls-lisp-use-insert-directory-program (funcall original-insert-directory
file switches wildcard full-directory-p) (let ((handler (find-file-name-handler
(expand-file-name file) (quote insert-directory))) (orig-file file)
wildcard-regexp) (if handler (funcall handler (quote insert-directory) file
switches wildcard full-directory-p) (if (string-match "--dired " switches) (setq
switches (replace-match "" nil nil switches))) (setq switches (delete 32 (delete
45 (append switches nil)))) (if (and ls-lisp-support-shell-wildcards
(string-match "[[?*]" file) (not (file-exists-p file))) (progn (or (not (eq ...
47)) (setq file (substring file 0 ...))) (setq wildcard t))) (if wildcard (setq
wildcard-regexp (if ls-lisp-support-shell-wildcards (wildcard-to-regexp
(file-name-nondirectory file)) (file-name-nondirectory file)) file
(file-name-directory file)) (if (memq 66 switches) (setq wildcard-regexp
"[^~]\\'"))) (condition-case err (ls-lisp-insert-directory file switches
(ls-lisp-time-index switches) wildcard-regexp full-directory-p) (invalid-regexp
(if (equal (cadr err) "Unmatched [ or [^") (progn (setq wildcard-regexp ... file
...) (ls-lisp-insert-directory file switches ... nil full-directory-p)) (signal
(car err) (cdr err))))) (save-excursion (goto-char (point-min)) (when
(re-search-forward "^total" nil t) (let ((available ...)) (when available
(replace-match "total used in directory") (end-of-line) (insert " available "
available))))))))
insert-directory("" "-al" nil nil)
dired-insert-directory("c:/drews-lisp-20/foobar" "-al" ("111.el" "") nil t)
dired-readin-insert()
dired-readin()
dired-internal-noselect(("c:/drews-lisp-20/foobar" "111.el" "") nil)
dired-noselect(("foobar" "111.el" "") nil)
dired(("foobar" "111.el" ""))
eval((dired (quote ("foobar" "111.el" ""))) nil)
eval-expression((dired (quote ("foobar" "111.el" ""))) nil)
call-interactively(eval-expression nil nil)
But I already pointed you to the code that needs to be changed.
And I already provided the fix:
> `ls-lisp-insert-directory' should in fact do nothing at all (no-op) if
> FILE is "". The entire body should be wrapped in this:
> (when (> (length file) 0) ...)
Simply wrap the body of `ls-lisp-insert-directory' with (when (> (length file)
0) ...), and the problem is fixed. And that is the right way to fix it.
^ permalink raw reply [flat|nested] 6+ messages in thread* bug#7112: 24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
2011-07-13 15:13 ` Drew Adams
@ 2011-08-02 20:06 ` Lars Magne Ingebrigtsen
2011-08-02 21:25 ` Drew Adams
0 siblings, 1 reply; 6+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-08-02 20:06 UTC (permalink / raw)
To: Drew Adams; +Cc: 7112
"Drew Adams" <drew.adams@oracle.com> writes:
>> > (error "Args out of range: \"\", -1")
>> > The reason is the following code near the end of
>> > `ls-lisp-insert-directory':
>>
>> I think this should most likely be fixed in the caller (i.e., in
>> dired.el).
>
> What does that mean? dired.el is a file, not a caller.
dired.el should not be asking ls-lisp to list a directory called "",
which is obviously something that should generate an error.
>> What's the backtrace for this bug?
>
> Why not try it yourself, using the emacs -Q recipe:
>
> M-: (dired '("foobar" "111.el" ""))
Why would you call this function with an empty string as a parameter?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog http://lars.ingebrigtsen.no/
^ permalink raw reply [flat|nested] 6+ messages in thread* bug#7112: 24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
2011-08-02 20:06 ` Lars Magne Ingebrigtsen
@ 2011-08-02 21:25 ` Drew Adams
2011-08-02 21:32 ` Lars Magne Ingebrigtsen
0 siblings, 1 reply; 6+ messages in thread
From: Drew Adams @ 2011-08-02 21:25 UTC (permalink / raw)
To: 'Lars Magne Ingebrigtsen'; +Cc: 7112
> dired.el should not be asking ls-lisp to list a directory called "",
> which is obviously something that should generate an error.
`ls-lisp-insert-directory' should not raise a low-level, Args out of range
error. It should itself DTRT for an empty file name. Maybe it should raise an
error, but not that low-level error. Or maybe it should, as the Subject line
suggests, ignore empty file names. I obviously think the latter is preferable,
but I suppose it's open for discussion.
> >> What's the backtrace for this bug?
> >
> > Why not try it yourself, using the emacs -Q recipe:
> >
> > M-: (dired '("foobar" "111.el" ""))
>
> Why would you call this function with an empty string as a parameter?
Because you can? No experienced programmer takes refuge behind the argument
"Why would anyone ever do that?" or "Don't worry; no one would ever do that."
Sooner or later programmers learn that users will do anything they can, and code
should be prepared. There is never any value in making excuses that blame
"stupid" users (or "stupid" caller code).
^ permalink raw reply [flat|nested] 6+ messages in thread* bug#7112: 24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
2011-08-02 21:25 ` Drew Adams
@ 2011-08-02 21:32 ` Lars Magne Ingebrigtsen
0 siblings, 0 replies; 6+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-08-02 21:32 UTC (permalink / raw)
To: Drew Adams; +Cc: 7112-close
Drew Adams <drew.adams@oracle.com> writes:
> `ls-lisp-insert-directory' should not raise a low-level, Args out of
> range error. It should itself DTRT for an empty file name.
Which is to give an error. It's a nonsensical input.
>> > Why not try it yourself, using the emacs -Q recipe:
>> >
>> > M-: (dired '("foobar" "111.el" ""))
>>
>> Why would you call this function with an empty string as a parameter?
>
> Because you can? No experienced programmer takes refuge behind the argument
> "Why would anyone ever do that?" or "Don't worry; no one would ever do that."
It gives an error. If you call `dired' with `t', you get
Debugger entered--Lisp error: (wrong-type-argument stringp t)
file-name-as-directory(t)
dired-noselect(t nil)
Don't do that, then. I'm closing this report.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog http://lars.ingebrigtsen.no/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-02 21:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-27 0:21 bug#7112: 24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE Drew Adams
2011-07-13 13:18 ` Lars Magne Ingebrigtsen
2011-07-13 15:13 ` Drew Adams
2011-08-02 20:06 ` Lars Magne Ingebrigtsen
2011-08-02 21:25 ` Drew Adams
2011-08-02 21:32 ` Lars Magne Ingebrigtsen
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.