Re: [PATCH 2/2] shell: new no-interactive-login command to print a custom message

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Mar 10, 2013, 10:49 UTC
Message-ID: <CALkWK0kK3YCwkv26cxVf61yUd8WHmHDG+mFwb2VRwNF3k_40qA@mail.gmail.com>
In-Reply-To: <20130309220011.GC24777@elie.Belkin>

Jonathan Nieder wrote:

Show 8 quoted lines

>  * If the file ~/git-shell-commands/no-interactive-login exists,
>    run no-interactive-login to let the server say what it likes,
>    then hang up.
>
>  * Otherwise, if ~/git-shell-commands/ is present, start an
>    interactive read-eval-print loop.
>
>  * Otherwise, print the usual configuration hint and hang up.

Excellent. A way to suppress the ugly warning, and replace it with a nice message in a non-interactive shell. You've chosen "no-interactive-login" as the name of this special file, which is reasonable. I'm not too fond of the name "git-shell-commands" in the first place, but I suspect it's too late to do anything about it now.

Show 19 quoted lines

> diff --git a/shell.c b/shell.c
> index 84b237fe..1429870a 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -6,6 +6,7 @@
>
>  #define COMMAND_DIR "git-shell-commands"
>  #define HELP_COMMAND COMMAND_DIR "/help"
> +#define NOLOGIN_COMMAND COMMAND_DIR "/no-interactive-login"
>
>  static int do_generic_cmd(const char *me, char *arg)
>  {
> @@ -65,6 +66,18 @@ static void run_shell(void)
>  {
>         int done = 0;
>         static const char *help_argv[] = { HELP_COMMAND, NULL };
> +
> +       if (!access(NOLOGIN_COMMAND, F_OK)) {
> +               /* Interactive login disabled. */

You're just checking for its existence here, not for execute permissions.

> +               const char *argv[] = { NOLOGIN_COMMAND, NULL };
> +               int status;
> +
> +               status = run_command_v_opt(argv, 0);

If "no-interactive-login" doesn't have execute permissions, we'll get an error from here:

    fatal: cannot exec 'git-shell-commands/no-interactive-login':
Permission denied

Would you like to check that the file has execute permission in advance to prevent some extra processing (in run_command_v_opt, start_command and friends) before this message is printed?

Looks good otherwise.