diff options
author | Jeff King <peff@peff.net> | 2022-08-30 23:40:56 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2022-08-31 00:31:37 +0300 |
commit | 0682bc43f5a48bbd4152e9954e647351a0fb9fe9 (patch) | |
tree | eb254d5d0ffeca7ea2b43b8b8db893f04f211149 | |
parent | ee69e7884e0cae3d2feabd5fcce8d3dfb44dda3a (diff) |
test-crontab: minor memory and error handling fixes
Since ee69e7884e (gc: use temporary file for editing crontab,
2022-08-28), we now insist that "argc == 3" (and otherwise return an
error). Coverity notes that this causes some dead code:
if (argc == 3)
fclose(from);
else
fclose(to);
as we will never trigger the else. This also causes a memory leak, since
we'll never close "to".
Now that all paths require 2 arguments, we can just reorganize the
function to check argc up front, and tweak the cleanup to do the right
thing for all cases.
While we're here, we can also notice some minor problems:
- we return a negative int via error() from what is essentially a
main() function; we should return a positive non-zero value for
error. Or better yet, we can just use usage(), which gives a better
message.
- while writing the usage message, we can note the one in the comment
was made out of date by ee69e7884e. But it also had a typo already,
calling the subcommand "cron" and not "crontab"
- we didn't check for an error from fopen(), meaning we would segfault
if the to-be-read file was missing. We can use xfopen() to catch
this.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r-- | t/helper/test-crontab.c | 23 |
1 files changed, 12 insertions, 11 deletions
diff --git a/t/helper/test-crontab.c b/t/helper/test-crontab.c index 2942543046..e6c1b1e22b 100644 --- a/t/helper/test-crontab.c +++ b/t/helper/test-crontab.c @@ -2,33 +2,34 @@ #include "cache.h" /* - * Usage: test-tool cron <file> [-l] + * Usage: test-tool crontab <file> -l|<input> * * If -l is specified, then write the contents of <file> to stdout. - * Otherwise, write from stdin into <file>. + * Otherwise, copy the contents of <input> into <file>. */ int cmd__crontab(int argc, const char **argv) { int a; FILE *from, *to; - if (argc == 3 && !strcmp(argv[2], "-l")) { + if (argc != 3) + usage("test-tool crontab <file> -l|<input>"); + + if (!strcmp(argv[2], "-l")) { from = fopen(argv[1], "r"); if (!from) return 0; to = stdout; - } else if (argc == 3) { - from = fopen(argv[2], "r"); - to = fopen(argv[1], "w"); - } else - return error("unknown arguments"); + } else { + from = xfopen(argv[2], "r"); + to = xfopen(argv[1], "w"); + } while ((a = fgetc(from)) != EOF) fputc(a, to); - if (argc == 3) - fclose(from); - else + fclose(from); + if (to != stdout) fclose(to); return 0; |