Skip to content

Commit 0682bc4

Browse files
peffgitster
authored andcommitted
test-crontab: minor memory and error handling fixes
Since ee69e78 (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 ee69e78. 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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ee69e78 commit 0682bc4

File tree

1 file changed

+12
-11
lines changed

1 file changed

+12
-11
lines changed

t/helper/test-crontab.c

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,33 +2,34 @@
22
#include "cache.h"
33

44
/*
5-
* Usage: test-tool cron <file> [-l]
5+
* Usage: test-tool crontab <file> -l|<input>
66
*
77
* If -l is specified, then write the contents of <file> to stdout.
8-
* Otherwise, write from stdin into <file>.
8+
* Otherwise, copy the contents of <input> into <file>.
99
*/
1010
int cmd__crontab(int argc, const char **argv)
1111
{
1212
int a;
1313
FILE *from, *to;
1414

15-
if (argc == 3 && !strcmp(argv[2], "-l")) {
15+
if (argc != 3)
16+
usage("test-tool crontab <file> -l|<input>");
17+
18+
if (!strcmp(argv[2], "-l")) {
1619
from = fopen(argv[1], "r");
1720
if (!from)
1821
return 0;
1922
to = stdout;
20-
} else if (argc == 3) {
21-
from = fopen(argv[2], "r");
22-
to = fopen(argv[1], "w");
23-
} else
24-
return error("unknown arguments");
23+
} else {
24+
from = xfopen(argv[2], "r");
25+
to = xfopen(argv[1], "w");
26+
}
2527

2628
while ((a = fgetc(from)) != EOF)
2729
fputc(a, to);
2830

29-
if (argc == 3)
30-
fclose(from);
31-
else
31+
fclose(from);
32+
if (to != stdout)
3233
fclose(to);
3334

3435
return 0;

0 commit comments

Comments
 (0)