From 07fba63e9eda509d1eee13df0b325dbd4be2f3cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=2E=20David=20Ib=C3=A1=C3=B1ez?= Date: Sat, 6 Jul 2013 23:51:40 +0200 Subject: [PATCH 1/3] Fix return value in git_config_get_multivar If there is not an error, the return value was always the return value of the last call to file->get_multivar With this commit GIT_ENOTFOUND is only returned if all the calls to filge-get_multivar return GIT_ENOTFOUND. --- src/config.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/config.c b/src/config.c index 068c40260..aaad7d87c 100644 --- a/src/config.c +++ b/src/config.c @@ -535,6 +535,7 @@ int git_config_get_multivar( file_internal *internal; git_config_backend *file; int ret = GIT_ENOTFOUND; + int err; size_t i; /* @@ -547,9 +548,15 @@ int git_config_get_multivar( continue; file = internal->file; - ret = file->get_multivar(file, name, regexp, cb, payload); - if (ret < 0 && ret != GIT_ENOTFOUND) - return ret; + err = file->get_multivar(file, name, regexp, cb, payload); + switch (err) { + case GIT_OK: + ret = GIT_OK; + case GIT_ENOTFOUND: + break; + default: + return err; + } } return (ret == GIT_ENOTFOUND) ? config_error_notfound(name) : 0; From 7b5c0d18460b6cb0a65543b92002a29644dbb458 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 9 Jul 2013 16:45:23 -0700 Subject: [PATCH 2/3] Add more tests for git_config_get_multivar The old tests didn't try failing lookups or lookups across multiple config files with some having the pattern and some not having it. --- tests-clar/config/multivar.c | 56 ++++++++++++++++++++++++---- tests-clar/resources/config/config11 | 2 +- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/tests-clar/config/multivar.c b/tests-clar/config/multivar.c index 0bda6bcec..efc431502 100644 --- a/tests-clar/config/multivar.c +++ b/tests-clar/config/multivar.c @@ -1,6 +1,6 @@ #include "clar_libgit2.h" -static const char *_name = "remote.fancy.url"; +static const char *_name = "remote.ab.url"; void test_config_multivar__initialize(void) { @@ -46,20 +46,60 @@ static int cb(const git_config_entry *entry, void *data) return 0; } +static void check_get_multivar( + git_config *cfg, int expected, int expected_patterned) +{ + int n = 0; + + if (expected > 0) { + cl_git_pass(git_config_get_multivar(cfg, _name, NULL, cb, &n)); + cl_assert_equal_i(expected, n); + } else { + cl_assert_equal_i(GIT_ENOTFOUND, + git_config_get_multivar(cfg, _name, NULL, cb, &n)); + } + + n = 0; + + if (expected_patterned > 0) { + cl_git_pass(git_config_get_multivar(cfg, _name, "example", cb, &n)); + cl_assert_equal_i(expected_patterned, n); + } else { + cl_assert_equal_i(GIT_ENOTFOUND, + git_config_get_multivar(cfg, _name, "example", cb, &n)); + } +} + void test_config_multivar__get(void) { git_config *cfg; - int n; cl_git_pass(git_config_open_ondisk(&cfg, "config/config11")); + check_get_multivar(cfg, 2, 1); - n = 0; - cl_git_pass(git_config_get_multivar(cfg, _name, NULL, cb, &n)); - cl_assert(n == 2); + /* add another that has the _name entry */ + cl_git_pass(git_config_add_file_ondisk(cfg, "config/config9", GIT_CONFIG_LEVEL_SYSTEM, 1)); + check_get_multivar(cfg, 3, 2); - n = 0; - cl_git_pass(git_config_get_multivar(cfg, _name, "example", cb, &n)); - cl_assert(n == 1); + /* add another that does not have the _name entry */ + cl_git_pass(git_config_add_file_ondisk(cfg, "config/config0", GIT_CONFIG_LEVEL_GLOBAL, 1)); + check_get_multivar(cfg, 3, 2); + + /* add another that does not have the _name entry at the end */ + cl_git_pass(git_config_add_file_ondisk(cfg, "config/config1", GIT_CONFIG_LEVEL_APP, 1)); + check_get_multivar(cfg, 3, 2); + + /* drop original file */ + cl_git_pass(git_config_add_file_ondisk(cfg, "config/config2", GIT_CONFIG_LEVEL_LOCAL, 1)); + check_get_multivar(cfg, 1, 1); + + /* drop other file with match */ + cl_git_pass(git_config_add_file_ondisk(cfg, "config/config3", GIT_CONFIG_LEVEL_SYSTEM, 1)); + check_get_multivar(cfg, 0, 0); + + /* reload original file (add different place in order) */ + cl_git_pass(git_config_add_file_ondisk(cfg, "config/config11", GIT_CONFIG_LEVEL_SYSTEM, 1)); + check_get_multivar(cfg, 2, 1); git_config_free(cfg); } diff --git a/tests-clar/resources/config/config11 b/tests-clar/resources/config/config11 index 7331862a5..1d8a74470 100644 --- a/tests-clar/resources/config/config11 +++ b/tests-clar/resources/config/config11 @@ -1,4 +1,4 @@ -[remote "fancy"] +[remote "ab"] url = git://github.com/libgit2/libgit2 url = git://git.example.com/libgit2 From e4fda954d6d914609498fc3bcbd27b4e2b5834d3 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 9 Jul 2013 16:46:18 -0700 Subject: [PATCH 3/3] A little git_config_get_multivar code cleanup --- src/config.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/config.c b/src/config.c index aaad7d87c..2a058549f 100644 --- a/src/config.c +++ b/src/config.c @@ -534,8 +534,7 @@ int git_config_get_multivar( { file_internal *internal; git_config_backend *file; - int ret = GIT_ENOTFOUND; - int err; + int ret = GIT_ENOTFOUND, err; size_t i; /* @@ -548,15 +547,10 @@ int git_config_get_multivar( continue; file = internal->file; - err = file->get_multivar(file, name, regexp, cb, payload); - switch (err) { - case GIT_OK: - ret = GIT_OK; - case GIT_ENOTFOUND: - break; - default: - return err; - } + if (!(err = file->get_multivar(file, name, regexp, cb, payload))) + ret = 0; + else if (err != GIT_ENOTFOUND) + return err; } return (ret == GIT_ENOTFOUND) ? config_error_notfound(name) : 0;