Thank you this is very illuminating and makes it clear that my naive
one-line fix would be inappropriate. Given the current state of
affairs there is really no good reason for PATH to ever be unset or
set to empty vs. explicitly set to '.', right? So might be worth
spelling out in the docs that this is inadvisable :).
On Wed, Mar 8, 2023 at 7:31 PM Grisha Levit wrote:
>
> On Wed, Mar 8, 2023 at 4:28 PM Moshe Looks wrote:
> > This is very old code and I have no idea what the original intention
> > was in returning 'name' here or if doing so is still performing useful
> > work, but the tests pass at least.
>
> A null $PATH component is treated as the current directory, and bash
> treats an empty $PATH as having only a single null component, so we
> can't just fail to return a command when $PATH is empty.
>
> The current code's behavior of returning the path as provided when
> $PATH is empty accomplishes the goal of "finding" the path in $PATH
> but ignores any flags that would normally limit the results to files
> that exist/are executable/etc.
>
> This leads to weirdness not just with command_not_found_handle but
> with command name completion and `type -a' and `command output as
> well.
>
> $ cd /bin
> $ PATH= compgen -c -X '!bash'
> $ PATH= type -a bash
> -bash: type: bash: not found
>
> I think it might make sense to change code that looks at the value of
> PATH to explicitly treat an empty value as `.' so that all such
> behavior is consistent.
>
> diff --git a/bashline.c b/bashline.c
> index 3238b13a..e0d6107c 100644
> --- a/bashline.c
> +++ b/bashline.c
> @@ -2063,6 +2063,8 @@ command_word_completion_function (const char
> *hint_text, int state)
> dequoted_hint = bash_dequote_filename (hint, 0);
>
>path = get_string_value ("PATH");
> + if (path == 0 || *path == '\0')
> + path = ".";
>path_index = dot_in_path = 0;
>
>/* Initialize the variables for each type of command word. */
> diff --git a/findcmd.c b/findcmd.c
> index 186871ac..b413c870 100644
> --- a/findcmd.c
> +++ b/findcmd.c
> @@ -256,13 +256,13 @@ _find_user_command_internal (const char *name, int
> flags)
>
>/* Search for the value of PATH in both the temporary environments and
> in the regular list of variables. */
> - if (var = find_variable_tempenv ("PATH")) /* XXX could be array? */
> -path_list = value_cell (var);
> + if (var = find_variable_tempenv ("PATH"))
> +path_list = get_variable_value (var);
>else
> -path_list = (char *)NULL;
> +path_list = 0;
>
>if (path_list == 0 || *path_list == '\0')
> -return (savestring (name));
> +path_list = ".";
>
>cmd = find_user_command_in_path (name, path_list, flags, (int *)0);
>
> @@ -363,11 +363,14 @@ search_for_command (const char *pathname, int flags)
> {
>if (flags & CMDSRCH_STDPATH)
> path_list = conf_standard_path ();
> - else if (temp_path || path)
> - path_list = value_cell (path);
> + else if (path)
> + path_list = get_variable_value (path);
>else
> path_list = 0;
>
> + if (path_list == 0 || *path_list == '\0')
> + path_list = ".";
> +
>command = find_user_command_in_path (pathname, path_list,
> FS_EXEC_PREFERRED|FS_NODIRS, &st);
>
>if (command && hashing_enabled && temp_path == 0 && (flags &
> CMDSRCH_HASH))
> @@ -440,7 +443,9 @@ user_command_matches (const char *name, int flags,
> int state)
> if (stat (".", &dotinfo) < 0)
> dotinfo.st_dev = dotinfo.st_ino = 0; /* so same_file won't match */
> path_list = get_string_value ("PATH");
> -path_index = 0;
> + if (path_list == 0 || *path_list == '\0')
> + path_list = ".";
> + path_index = 0;
> }
>
>while (path_list && path_list[path_index])
