GNU bug report logs - #7112
24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 7112 in the body.
You can then email your comments to 7112 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: <bug-gnu-emacs <at> gnu.org>
Subject: 24.0.50;
	[PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
Date: Sun, 26 Sep 2010 17:21:58 -0700

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'
 

Message #8 received at 7112 <at> debbugs.gnu.org (full text, mbox):

From: Lars Magne Ingebrigtsen <larsi <at> gnus.org>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 7112 <at> debbugs.gnu.org
Subject: Re: 24.0.50;
	[PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
Date: Wed, 13 Jul 2011 15:18:43 +0200

"Drew Adams" <drew.adams <at> 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/

Message #11 received at 7112 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Lars Magne Ingebrigtsen'" <larsi <at> gnus.org>
Cc: 7112 <at> debbugs.gnu.org
Subject: RE: 24.0.50;
	[PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
Date: Wed, 13 Jul 2011 08:13:59 -0700

> > 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.

Message #14 received at 7112 <at> debbugs.gnu.org (full text, mbox):

From: Lars Magne Ingebrigtsen <larsi <at> gnus.org>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 7112 <at> debbugs.gnu.org
Subject: Re: 24.0.50;
	[PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
Date: Tue, 02 Aug 2011 22:06:46 +0200

"Drew Adams" <drew.adams <at> 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/

Message #17 received at 7112 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: "'Lars Magne Ingebrigtsen'" <larsi <at> gnus.org>
Cc: 7112 <at> debbugs.gnu.org
Subject: RE: 24.0.50; [PATCH] `ls-lisp-insert-directory' should be no-op for
	empty FILE
Date: Tue, 2 Aug 2011 14:25:08 -0700 (PDT)

> 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).

Message #22 received at 7112-close <at> debbugs.gnu.org (full text, mbox):

From: Lars Magne Ingebrigtsen <larsi <at> gnus.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 7112-close <at> debbugs.gnu.org
Subject: Re: 24.0.50;
	[PATCH] `ls-lisp-insert-directory' should be no-op for empty FILE
Date: Tue, 02 Aug 2011 23:32:07 +0200

Drew Adams <drew.adams <at> 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/

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.