zsh-workers
 help / color / mirror / code / Atom feed
From: Bart Schaefer <schaefer@brasslantern.com>
To: zsh workers <zsh-workers@zsh.org>
Subject: Re: [RFC or so] Add HASH_LOOKUP option
Date: Mon, 23 Aug 2010 10:46:10 -0700	[thread overview]
Message-ID: <100823104610.ZM26959@torch.brasslantern.com> (raw)
In-Reply-To: <AANLkTim0NXjt4hp-UV4PK_7wNS2+tvf8NOu9W8F9RUSD@mail.gmail.com>

On Aug 23,  4:09pm, Mikael Magnusson wrote:
}
} When this is unset, external commands are always resolved with a full
} path search, but still inserted into the hash for spell correction if
} those options are on.
} 
} diff --git a/Src/exec.c b/Src/exec.c
} index 93d1b26..9a488fe 100644
} --- a/Src/exec.c
} +++ b/Src/exec.c
} @@ -754,7 +754,7 @@ findcmd(char *arg0, int docopy)
}  	    }
}  	    break;
}  	}
} -    if (cn) {
} +    if (cn && isset(HASHLOOKUP)) {
}  	char nn[PATH_MAX];
} 
}  	if (cn->node.flags & HASHED)

I think there may be  problem with this in the event that the "hash"
command has been used to deliberately insert an entry into the hash
table.  That's a documented mechanism for overriding path search.
Also I think you've missed that execute() also uses the hash table;
did you intend that "command foo" ignores NO_HASHLOOKUP?

In looking more closely, there are a number of things about findcmd()
that look a bit fishy to me.  Correct me where I've gone wrong?

It starts (if the command is not already in the hash table and HASHCMDS
is set) by doing hashcmd(), which performs a search of only absolute
directory names in the path (not, e.g., ".", "..", or other relative
names).  That search itself seems a bit off because if *arg0 == '/'
then it's going to prepend the path component anyway. [*]

It then ignores what it got back from the search and, if arg0 contains
a '/', checks whether arg0 is executable without path traversal.  This
seems a bit sideways to me; why not do that first?  HASHDIRS, I guess?
[This disagrees with execute(), which does the directly-executable test
first.]  In any case this can return NULL, leaving the ignored results
from the previous search in the hash table, unless PATHDIRS is set.

Then, if it DID find the command either in the hashtable already or
via hashcmd() but the hashtable entry does not have the HASHED bit [**]
it performs a search of only relative directory names in the path ahead
of the previously-found absolute directory.

Finally, if all of the above failed, we do a full path search of both
relative and absolute directories again, which is redundant in the
case of HASHCMDS unless by some unlikely race condition the command
has appeared in one of the directories in the path in the meantime.


[*] And indeed, from zsh -f:

torch% hash
torch% print =/X11/xterm
zsh: /X11/xterm not found
torch% hash | grep X11
/X11/xterm=/usr/bin//X11/xterm


[**] The HASHED bit means the command was deliberately inserted into
the hash table with the "hash" builtin, rather than found by search.
In this case the both findcmd() and execute() are forced to believe
what the hash table tells them.


  parent reply	other threads:[~2010-08-23 17:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-23 14:09 Mikael Magnusson
2010-08-23 14:55 ` Peter Stephenson
2010-08-23 17:46 ` Bart Schaefer [this message]
2010-08-23 18:25   ` Mikael Magnusson
2010-08-24  8:37     ` Bart Schaefer
2011-06-02 15:11   ` Mikael Magnusson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=100823104610.ZM26959@torch.brasslantern.com \
    --to=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

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 NNTP newsgroup(s).