Re: [PATCH v6 3/6] run-command: make `exists_in_PATH()` non-static

[Junio C Hamano <gitster@pobox.com>]

Miriam Rubio <mirucam@gmail.com> writes:

> From: Pranit Bauva <pranit.bauva@gmail.com>
>
> Removes the `static` keyword from `exists_in_PATH()` function
> and declares the function in `run-command.h` file.

"Remove" and "declare", as if we are giving an order to somebody
else to make these changes.

> The function will be used in bisect_visualize() in a later
> commit.
>
> Mentored by: Christian Couder <chriscool@tuxfamily.org>
> Mentored by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
> Signed-off-by: Miriam Rubio <mirucam@gmail.com>
> ---
>  run-command.c |  2 +-
>  run-command.h | 12 ++++++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index f72e72cce7..390f46819f 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -210,7 +210,7 @@ static char *locate_in_PATH(const char *file)
>  	return NULL;
>  }
>  
> -static int exists_in_PATH(const char *file)
> +int exists_in_PATH(const char *file)
>  {
>  	char *r = locate_in_PATH(file);
>  	int found = r != NULL;
> diff --git a/run-command.h b/run-command.h
> index af1296769f..54d74b706f 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -182,6 +182,18 @@ void child_process_clear(struct child_process *);
>  
>  int is_executable(const char *name);
>  
> +/**
> + * Search if a $PATH for a command exists.  This emulates the path search that

The first sentence does not make sense to me.  Isn't this for
checking if a command exists in one of the directories on $PATH?

	Check if the command exists on $PATH.

may make more sense, especially since "search" may hint that the
caller may be able to learn where it exists, which is not the case.

> + * execvp would perform, without actually executing the command so it
> + * can be used before fork() to prepare to run a command using
> + * execve() or after execvp() to diagnose why it failed.
> + *
> + * The caller should ensure that file contains no directory separators.

Consistently use "command" instead of "file" and rename the
parameter in the prototype below from "file" to "command".

Alternatively, you can rewrite the first paragraph above to make
sure that it is clear to the readers that "command" it refers to is
actually the "file" parameter the function takes.  A rewrite of the
first sentence I just rewrote above may become

	Check if an executable "file" exists on $PATH.

which does not look too bad, but "executing the file so it can ..."
and "to run a file using..." smell a bit strange, and that is why I
suggested to consistently use "command" instead.

> + *
> + * Returns 1 if it is found in $PATH or 0 if the command could not be found.
> + */
> +int exists_in_PATH(const char *file);

Thanks.