Re: [PATCH] maintenance: specify explicit stdin for crontab

From: Martin Ågren <martin.agren@gmail.com>
Date: Mar 30, 2021, 05:41 UTC
Message-ID: <CAN0heSrSNJhy33Wi9Yq8kfnkJEyvQoadyj8joLqHtV+SYPs1sw@mail.gmail.com>
In-Reply-To: <20210329210928.561586-1-me@ikke.info>

On Mon, 29 Mar 2021 at 23:23, Kevin Daudt <me@ikke.info> wrote:

>
> There are multiple crontab implementations that require stdin for
> editing a crontab to be explicitly specified as '-'.

[...]

Show 10 quoted lines

> --- a/t/helper/test-crontab.c
> +++ b/t/helper/test-crontab.c
> @@ -17,7 +17,7 @@ int cmd__crontab(int argc, const char **argv)
>                 if (!from)
>                         return 0;
>                 to = stdout;
> -       } else if (argc == 2) {
> +       } else if ((argc == 3 && !strcmp(argv[2], "-")) || argc == 2) {
>                 from = stdin;
>                 to = fopen(argv[1], "w");

Would it make sense to make this

  } else if (argc == 3 && !strcmp(argv[2], "-")) {

in order to make this test-tool as picky as possible and to only accept the kind of usage we want to (well, need to) use? The tests as they stand would still pass, which I think argues for us not really needing that "argc == 2".

This would be followed by

  } else
          return error("unknown arguments");

which wouldn't be super helpful if you forgot the "-", but helpful enough for an internal test-tool, I guess.

Speaking of usage and hints, there's "Usage: ..." in a comment at the top of this file. It should probably be updated either way.

Martin