command-spelling correction strangeness

command-spelling correction strangeness

[Paul Kimoto]

This is zsh-3.1.6-pws-{5,6} on i586-pc-linux-gnu, with glibc-2.1.2.
(I think that this also occurred under zsh-3.1.6.)

I have a non-executable, non-zero-size file "ps"
(-rw-r--r--   1 kimoto   kimoto        268 Mar 30 16:44 ps)
in a directory ahead of /bin in PATH.  When I have the "correct" and
"autocd" options set, often zsh offers to correct "ps" to "ls" _after_ 
"ps" has been run once.

I can reliably reproduce this under "zsh -f" as follows:

### begin example

perdita[0,821]: zsh -f
perdita% setopt correct autocd
perdita% ps
  PID TTY          TIME CMD
  421 ttyp1    00:00:19 zsh 
  565 ttyp1    00:00:00 tail
20787 ttyp1    00:00:05 emacs
21382 ttyp1    00:00:00 less 
21422 ttyp1    00:00:00 zsh  
21423 ttyp1    00:00:00 ps   
perdita% ps
zsh: correct 'ps' to 'ls' [nyae]? n
  PID TTY          TIME CMD
  421 ttyp1    00:00:19 zsh  
  565 ttyp1    00:00:00 tail 
20787 ttyp1    00:00:05 emacs
21382 ttyp1    00:00:00 less 
21422 ttyp1    00:00:00 zsh  
21424 ttyp1    00:00:00 ps  

### end example

I am not a zsh-workers subscriber, so please cc me as
appropriate.    

        -Paul <kimoto@lightlink.com>

Re: command-spelling correction strangeness

[Bart Schaefer]

On Sep 28, 11:56pm, Paul Kimoto wrote:
} Subject: command-spelling correction strangeness
}
} I have a non-executable, non-zero-size file "ps"
} (-rw-r--r--   1 kimoto   kimoto        268 Mar 30 16:44 ps)
} in a directory ahead of /bin in PATH.  When I have the "correct" and
} "autocd" options set, often zsh offers to correct "ps" to "ls" _after_ 
} "ps" has been run once.

A change was made about a year ago (zsh-workers/4404 and follow-ups) to
remove entries for non-executable files from the command hash table.
This happens only when "autocd" is set, because the only bad side effect
of bogus hash table entries is that if they're directories rather than
non-executable files, you can't autocd into them.

However, if you have the hashcmds option set, the rest of the path is
searched and the correct location gets immediately added back again --
so you do not set hashcmds.  Am I correct?

Here's what happens:

Running any external command causes the command hash table to be filled.
Because the non-executable "ps" is in $PATH ahead of /bin, it gets added
to the hash table and /bin/ps does not.

Later, you run "ps".  The hash entry for "ps" is found to point to the
non-executable file and is therefore deleted from the hash table.  Now
there is no entry for "ps" at all; "/bin/ps" is found by re-searching the
path, but this is not added to the hash table.

Finally, you run "ps" again.  Zsh looks in the hash table and doesn't
find it, so it picks the next closest thing (for a keyboard-oriented
definition of `closest') and asks whether it should correct to that.  On
my system it chooses "psf" which is some kind of postscript print filter,
but on your system it must be "ls".

So ... what you describe may or may not be a bug, but the following should
cause the command table to be updated more intelligently.  I'd be just as
happy with encouraging people not to use "correct" without "hashcmds" ...

Index: Src/exec.c
===================================================================
@@ -1697,14 +1697,16 @@
 	if (!hn) {
 	    /* Resolve external commands */
 	    char *cmdarg = (char *) peekfirst(args);
+	    int dohashcmd = isset(HASHCMDS);
 
 	    hn = cmdnamtab->getnode(cmdnamtab, cmdarg);
 	    if (hn && trycd && !isreallycom((Cmdnam)hn)) {
 		cmdnamtab->removenode(cmdnamtab, cmdarg);
 		cmdnamtab->freenode(hn);
 		hn = NULL;
+		dohashcmd = 1;
 	    }
-	    if (!hn && isset(HASHCMDS) && strcmp(cmdarg, "..")) {
+	    if (!hn && dohashcmd && strcmp(cmdarg, "..")) {
 		for (s = cmdarg; *s && *s != '/'; s++);
 		if (!*s)
 		    hn = (HashNode) hashcmd(cmdarg, pathchecked);


-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: command-spelling correction strangeness

[Paul Kimoto]

[This is a copy of a message sent to Bart Schaefer
<schaefer@candle.brasslantern.com> that I Cc'ed to zsh-workers
before remembering that I was on a ORBS-tainted site whose mail
is rejected by sunsite.auc.dk.]

On Wed, Sep 29, 1999 at 05:39:43AM +0000, Bart Schaefer wrote:
> A change was made about a year ago (zsh-workers/4404 and follow-ups) to
> remove entries for non-executable files from the command hash table.
> This happens only when "autocd" is set, because the only bad side effect
> of bogus hash table entries is that if they're directories rather than
> non-executable files, you can't autocd into them.
> 
> However, if you have the hashcmds option set, the rest of the path is
> searched and the correct location gets immediately added back again --
> so you do not set hashcmds.  Am I correct?

[detailed explanation snipped]

Yes (as you may have seen in my "zsh -f" demonstration).  The explanation
is plausible.  I am now a little mystified by the documentation where it
says:

: HASH_CMDS <D>
:      Note the location of each command the first time it is executed.
:      Subsequent invocations of the same command will use the saved
:      location, avoiding a path search.  If this option is unset, no
                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
:      path hashing will be done at all.
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

(I admit that I have read only the documentation, not the source code.)

> So ... what you describe may or may not be a bug, but the following should
> cause the command table to be updated more intelligently.  I'd be just as
> happy with encouraging people not to use "correct" without "hashcmds" ...

It could be added to the documentation ....

[patch snipped]

	-Paul <kimoto@lightlink.com>

Re: command-spelling correction strangeness

[Bart Schaefer]

On Sep 29,  2:02am, Paul Kimoto wrote:
} Subject: Re: command-spelling correction strangeness
}
} On Wed, Sep 29, 1999 at 05:39:43AM +0000, Bart Schaefer wrote:
} > However, if you have the hashcmds option set, the rest of the path is
} > searched and the correct location gets immediately added back again --
} > so you do not set hashcmds.  Am I correct?
} 
} Yes (as you may have seen in my "zsh -f" demonstration).

That's actually puzzling, because when I run "zsh -f" for 3.1.6-pws-6,
all of hashcmds, hashdirs, and hashlistall are set.

Does your /etc/zshenv unset these, perhaps?

} I am now a little mystified by the documentation where it says:
} 
} : HASH_CMDS <D>
} :      Note the location of each command the first time it is executed.
} :      Subsequent invocations of the same command will use the saved
} :      location, avoiding a path search.  If this option is unset, no
}                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
} :      path hashing will be done at all.
}        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The manual is only partly correct, because ...

} > I'd be just as
} > happy with encouraging people not to use "correct" without "hashcmds" ...

... it turns out that the implementation of "correct" causes hashing to
occur, in which case the setting of hashdirs is respected independently
of hashcmds.

Given that "correct" silently causes hashing to occur, I think the patch
that I posted is a necessary one.  zsh-workers:  Should the doc be updated
to reflect that "correct" overrides "nohashcmds"?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: command-spelling correction strangeness

[Bart Schaefer]

On Oct 1, 12:26am, Paul Kimoto wrote:
} Subject: Re: command-spelling correction strangeness
}
} On Fri, Oct 01, 1999 at 04:02:27AM +0000, Bart Schaefer wrote:
} > Did you try my patch?  Did it make any difference to the behavior you
} > see?  [...]  I couldn't reproduce anything like what you
} > were seeing unless I unsetopt hashcmds.
}
} Your suspicion is right -- your patch doesn't make a difference: 
} sorry for misleading you.

OK, the real scoop:  My diagnosis was correct as far as it went, but I
missed one detail, which is that even when hashcmds is set it's possible
for `pathchecked' (zsh's internal pointer to how much of the path it has
searched) to already have advanced beyond the directory containing the
actual command.  In that case resuming the check where it left off is not
sufficient.

Zsh's internal Cmdnam structure holds a pointer to the spot in the path
where the command was found, so it's possible to restart the search from
there, and I actually had that implemented for a bit; but it occurred to
me that the reason the command can't be executed might be because it was
moved to another place in the path, in which event the search ought to be
started over from the beginning.  This works because hashing of a single
command does a test for executable-non-directory-ness, whereas hashing an
entire directory simply stuffs every entry into the table.  (So the whole
problem can be avoided in the first place by "unsetopt hashdirs".)

This goes on top of the previous patch.  Arguably, removing the hash node
could also be conditional on (flags & HASHED) -- that is, don't implicitly
remove it if the user explicitly hashed it -- but I can't think of any
good reason to deliberately hash a bogus command location.

Index: exec.c
===================================================================
RCS file: /extra/cvsroot/zsh/zsh-3.1/Src/exec.c,v
retrieving revision 1.59
diff -c -r1.59 exec.c
--- exec.c	1999/09/30 15:45:44	1.59
+++ exec.c	1999/10/01 16:19:30
@@ -1692,19 +1692,23 @@
 	if (!hn) {
 	    /* Resolve external commands */
 	    char *cmdarg = (char *) peekfirst(args);
+	    char **checkpath = pathchecked;
 	    int dohashcmd = isset(HASHCMDS);
 
 	    hn = cmdnamtab->getnode(cmdnamtab, cmdarg);
 	    if (hn && trycd && !isreallycom((Cmdnam)hn)) {
+		if (!(((Cmdnam)hn)->flags & HASHED)) {
+		    checkpath = path;
+		    dohashcmd = 1;
+		}
 		cmdnamtab->removenode(cmdnamtab, cmdarg);
 		cmdnamtab->freenode(hn);
 		hn = NULL;
-		dohashcmd = 1;
 	    }
 	    if (!hn && dohashcmd && strcmp(cmdarg, "..")) {
 		for (s = cmdarg; *s && *s != '/'; s++);
 		if (!*s)
-		    hn = (HashNode) hashcmd(cmdarg, pathchecked);
+		    hn = (HashNode) hashcmd(cmdarg, checkpath);
 	    }
 	}
 

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: command-spelling correction strangeness

[Paul Kimoto]

On Fri, Oct 01, 1999 at 04:32:03PM +0000, Bart Schaefer wrote:
> OK, the real scoop:  My diagnosis was correct as far as it went, but I
> missed one detail, which is that even when hashcmds is set it's possible
> for `pathchecked' (zsh's internal pointer to how much of the path it has
> searched) to already have advanced beyond the directory containing the
> actual command.  In that case resuming the check where it left off is not
> sufficient.
> 
> Zsh's internal Cmdnam structure holds a pointer to the spot in the path
> where the command was found, so it's possible to restart the search from
> there, and I actually had that implemented for a bit; but it occurred to
> me that the reason the command can't be executed might be because it was
> moved to another place in the path, in which event the search ought to be
> started over from the beginning.  This works because hashing of a single
> command does a test for executable-non-directory-ness, whereas hashing an
> entire directory simply stuffs every entry into the table.  (So the whole
> problem can be avoided in the first place by "unsetopt hashdirs".)
> 
> This goes on top of the previous patch.

[patch snipped]

The combination of this and the patch in
<990929053943.ZM22102@candle.brasslantern.com> 
(from 29 Sep 1999) does the job for me.

	-Paul <kimoto@lightlink.com>