]> git.ipfire.org Git - thirdparty/git.git/commitdiff
test-crontab: minor memory and error handling fixes
authorJeff King <peff@peff.net>
Tue, 30 Aug 2022 20:40:56 +0000 (16:40 -0400)
committerJunio C Hamano <gitster@pobox.com>
Tue, 30 Aug 2022 21:31:37 +0000 (14:31 -0700)
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>
t/helper/test-crontab.c

index 29425430466692cac0f3aae29e9d0adac0d1805d..e6c1b1e22bb36da604af3238341f6b7b14753796 100644 (file)
@@ -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;