Re: [PATCH v5 2/2] test-config: Add tests for the config_set API
From: Matthieu Moy <hidden>
Date: 2016-06-15 23:01:49
Tanay Abhra [off-list ref] writes:
quoted hunk
diff --git a/test-config.c b/test-config.c new file mode 100644 index 0000000..45ccd0a --- /dev/null +++ b/test-config.c@@ -0,0 +1,127 @@ +#include "cache.h" +#include "hashmap.h"
Useless include, you're not using the hashmap directly.
+int main(int argc, char **argv)
+{
+ int i, no_of_files;Unused no_of_files. With CFLAGS += -Wdeclaration-after-statement -Wall -Werror in config.mak, my git.git refuses to compile with your patch. You should have similar options for hacking on git.git.
+ if (argc == 3 && !strcmp(argv[1], "get_value")) {
You should do something like
if (argc < 2) {
fprintf(stderr, "Please, provide a command name on the command-line\n");
return 1;
}
before this. Otherwise, some argv[1] below are invalid on mis-use. No
need for thorough checks since it's just a test program, but better
avoid undefined behavior and segfaults anyway...
+ if (!git_config_get_value(argv[2], &v)) {
+ if (!v)
+ printf("(NULL)\n");
+ else
+ printf("%s\n", v);
+ return 0;
+ } else {
+ printf("Value not found for \"%s\"\n", argv[2]);
+ return -1;Avoid returning negative values from main. Your shell's $? won't see -1, but most likely 255 or so, but I think it even depends on the OS. You don't seem to use main's return for anything but error, so 0 = everything OK; 1 = some error is the common convention.
+ } else {
+ printf("Value not found for \"%s\"\n", argv[2]);
+ return -1;
+ }
+
+ } else if (!strcmp(argv[1], "configset_get_value_multi")) {Why a blank line before this "else if" and not before other "else if"s? -- Matthieu Moy http://www-verimag.imag.fr/~moy/