From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25510 invoked by alias); 24 Aug 2010 08:38:12 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 28197 Received: (qmail 24310 invoked from network); 24 Aug 2010 08:38:10 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received-SPF: none (ns1.primenet.com.au: domain at closedmail.com does not designate permitted sender hosts) From: Bart Schaefer Message-id: <100824013756.ZM27208@torch.brasslantern.com> Date: Tue, 24 Aug 2010 01:37:56 -0700 In-reply-to: Comments: In reply to Mikael Magnusson "Re: [RFC or so] Add HASH_LOOKUP option" (Aug 23, 8:25pm) References: <100823104610.ZM26959@torch.brasslantern.com> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh workers Subject: Re: [RFC or so] Add HASH_LOOKUP option MIME-version: 1.0 Content-type: text/plain; charset=us-ascii On Aug 23, 8:25pm, Mikael Magnusson wrote: } } > 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? } } As I said, I didn't check everywhere yet to see if there are more } duplicated codepaths, so no, that is not the intent. However, command } foo for me doesn't appear to ignore it for me. execute() gets called for all WC_SIMPLE commands that aren't builtins or functions, including the "command" and "exec" precommand modifiers: /* for command -p, search the default path */ if (defpath) { ... } else { if ((cn = (Cmdnam) cmdnamtab->getnode(cmdnamtab, arg0))) { ... } ... usual path search ... } } The fact that 'command' supposedly does a hash lookup while 'command } -p' doesn't seems to be rather undocumented too. It'd hardly be possible to search the default POSIX $PATH via a hash lookup, given that the hash table is filled from what is most commonly a non-default $path. } > 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. [*] } } That's the first fishy thing I noticed, why is this for loop over } $path copied to at least four places, two of which are in the same } function, with slight variations? My recollection (which I haven't bothered to augment by attempting to search the list archives) is that the path is searched first so that the hash table is populated so spelling corrections can be suggested, or something like that. The first such search, though, has to skip relative directories, lest "." cause NO_PATH_DIRS to be violated. Then it's necessary to repeat the search for just the directories that previously were skipped, in case one of them appears earlier in the path than the directory where the command was previously found. So that's two variations. Then execute() has to search either the default $PATH (third variant) or the normal $path (fourth variant), but doesn't have to worry about separating absolute and relative directories; it can just use whichever it finds first, just like the final fallback attempt in findcmd(). } I guess in this specific function I could move my isset(HASHLOOKUP) to } be in the & HASHED if() instead of the cn one. Which is to say if the } whole thing isn't broken in the first place. Except for the part about putting goofy stuff in the hash table in a few edge cases, I think it's not actually broken, just disorganized. } > [**] 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. } } Could this be used, theoretically, to still allow users to override } the path, and still do a full path search for search hashed entries? I'm not sure what you mean, but on my best guess: If you poke into the hash table with "hash foo=/xyz/foo" then the "foo" command will run /xyz/foo even if ${^path}/foo(N) is non-empty. However, and this might be considered a bug, changing the value of $path discards the entire hash table, including entries that have been explicitly inserted with "hash". } Another thing I was vaguely wondering about is how is HASH_CMDS forced } on by CORRECT? I was unable to grep up anything about it. It isn't actually forced on by CORRECT, but during the actual event of correcting, spckword() calls cmdnamtab->filltable() which has the same effect.