From 508a285ceab4523412546a92cd9444f8e78065e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= <l.s.r@web.de>
Date: Tue, 19 Jul 2016 21:05:43 +0200
Subject: [PATCH 1/4] submodule-config: use explicit empty string instead of
strbuf in config_from()
Use a string constant instead of an empty strbuf to shorten the code
and make it easier to read.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
submodule-config.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 7f67ec0c6a8..1210b267ddf 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -375,7 +375,6 @@ static const struct submodule *config_from(struct submodule_cache *cache,
const unsigned char *commit_sha1, const char *key,
enum lookup_type lookup_type)
{
- struct strbuf rev = STRBUF_INIT;
unsigned long config_size;
char *config;
unsigned char sha1[20];
@@ -426,7 +425,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
parameter.commit_sha1 = commit_sha1;
parameter.gitmodules_sha1 = sha1;
parameter.overwrite = 0;
- git_config_from_mem(parse_config, "submodule-blob", rev.buf,
+ git_config_from_mem(parse_config, "submodule-blob", "",
config, config_size, ¶meter);
free(config);
From 514dea905a3c72c8f7268347793f2947efd547be Mon Sep 17 00:00:00 2001
From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Thu, 28 Jul 2016 14:49:11 +0200
Subject: [PATCH 2/4] submodule-config: passing name reference for .gitmodule
blobs
Commit 959b5455 (submodule: implement a config API for lookup of
.gitmodules values, 2015-08-18) implemented the initial version of the
submodule config cache. During development of that initial version we
extracted the function gitmodule_sha1_from_commit(). During that process
we missed that the strbuf rev was still used in config_from() and now is
left empty. Lets fix this by also returning this string.
This means that now when reading .gitmodules from revisions, the error
messages also contain a reference to the blob they are from.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
submodule-config.c | 26 +++++++++++++++++---------
t/t7411-submodule-config.sh | 11 +++++++++++
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 1210b267ddf..853989e5586 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -349,9 +349,9 @@ static int parse_config(const char *var, const char *value, void *data)
}
static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
- unsigned char *gitmodules_sha1)
+ unsigned char *gitmodules_sha1,
+ struct strbuf *rev)
{
- struct strbuf rev = STRBUF_INIT;
int ret = 0;
if (is_null_sha1(commit_sha1)) {
@@ -359,11 +359,10 @@ static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
return 1;
}
- strbuf_addf(&rev, "%s:.gitmodules", sha1_to_hex(commit_sha1));
- if (get_sha1(rev.buf, gitmodules_sha1) >= 0)
+ strbuf_addf(rev, "%s:.gitmodules", sha1_to_hex(commit_sha1));
+ if (get_sha1(rev->buf, gitmodules_sha1) >= 0)
ret = 1;
- strbuf_release(&rev);
return ret;
}
@@ -375,6 +374,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
const unsigned char *commit_sha1, const char *key,
enum lookup_type lookup_type)
{
+ struct strbuf rev = STRBUF_INIT;
unsigned long config_size;
char *config;
unsigned char sha1[20];
@@ -397,8 +397,10 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return entry->config;
}
- if (!gitmodule_sha1_from_commit(commit_sha1, sha1))
+ if (!gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
+ strbuf_release(&rev);
return NULL;
+ }
switch (lookup_type) {
case lookup_name:
@@ -408,14 +410,19 @@ static const struct submodule *config_from(struct submodule_cache *cache,
submodule = cache_lookup_path(cache, sha1, key);
break;
}
- if (submodule)
+ if (submodule) {
+ strbuf_release(&rev);
return submodule;
+ }
config = read_sha1_file(sha1, &type, &config_size);
- if (!config)
+ if (!config) {
+ strbuf_release(&rev);
return NULL;
+ }
if (type != OBJ_BLOB) {
+ strbuf_release(&rev);
free(config);
return NULL;
}
@@ -425,8 +432,9 @@ static const struct submodule *config_from(struct submodule_cache *cache,
parameter.commit_sha1 = commit_sha1;
parameter.gitmodules_sha1 = sha1;
parameter.overwrite = 0;
- git_config_from_mem(parse_config, "submodule-blob", "",
+ git_config_from_mem(parse_config, "submodule-blob", rev.buf,
config, config_size, ¶meter);
+ strbuf_release(&rev);
free(config);
switch (lookup_type) {
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index fc97c3314e2..400e2b1439a 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -82,6 +82,17 @@ test_expect_success 'error in one submodule config lets continue' '
)
'
+test_expect_success 'error message contains blob reference' '
+ (cd super &&
+ sha1=$(git rev-parse HEAD) &&
+ test-submodule-config \
+ HEAD b \
+ HEAD submodule \
+ 2>actual_err &&
+ grep "submodule-blob $sha1:.gitmodules" actual_err >/dev/null
+ )
+'
+
cat >super/expect_url <<EOF
Submodule url: 'git@somewhere.else.net:a.git' for path 'b'
Submodule url: 'git@somewhere.else.net:submodule.git' for path 'submodule'
From 0918e25077cb9321011a973703cc597b078f0ab5 Mon Sep 17 00:00:00 2001
From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Thu, 28 Jul 2016 14:49:47 +0200
Subject: [PATCH 3/4] submodule-config: combine early return code into one goto
So we have simpler return handling code and all the cleanup code in
almost one place.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
submodule-config.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 853989e5586..a8875745909 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -376,7 +376,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
{
struct strbuf rev = STRBUF_INIT;
unsigned long config_size;
- char *config;
+ char *config = NULL;
unsigned char sha1[20];
enum object_type type;
const struct submodule *submodule = NULL;
@@ -397,10 +397,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return entry->config;
}
- if (!gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
- strbuf_release(&rev);
- return NULL;
- }
+ if (!gitmodule_sha1_from_commit(commit_sha1, sha1, &rev))
+ goto out;
switch (lookup_type) {
case lookup_name:
@@ -410,22 +408,12 @@ static const struct submodule *config_from(struct submodule_cache *cache,
submodule = cache_lookup_path(cache, sha1, key);
break;
}
- if (submodule) {
- strbuf_release(&rev);
- return submodule;
- }
+ if (submodule)
+ goto out;
config = read_sha1_file(sha1, &type, &config_size);
- if (!config) {
- strbuf_release(&rev);
- return NULL;
- }
-
- if (type != OBJ_BLOB) {
- strbuf_release(&rev);
- free(config);
- return NULL;
- }
+ if (!config || type != OBJ_BLOB)
+ goto out;
/* fill the submodule config into the cache */
parameter.cache = cache;
@@ -445,6 +433,11 @@ static const struct submodule *config_from(struct submodule_cache *cache,
default:
return NULL;
}
+
+out:
+ strbuf_release(&rev);
+ free(config);
+ return submodule;
}
static const struct submodule *config_from_path(struct submodule_cache *cache,
From 55cbe18e1146320674968820150126ee34e5d332 Mon Sep 17 00:00:00 2001
From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Thu, 28 Jul 2016 14:50:05 +0200
Subject: [PATCH 4/4] submodule-config: fix test binary crashing when no
arguments given
Since arg[0] will be NULL without any argument here and starts_with()
does not like NULL-pointers.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
test-submodule-config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test-submodule-config.c b/test-submodule-config.c
index dab8c277681..a4e4098a0fc 100644
--- a/test-submodule-config.c
+++ b/test-submodule-config.c
@@ -23,7 +23,7 @@ int main(int argc, char **argv)
arg++;
my_argc--;
- while (starts_with(arg[0], "--")) {
+ while (arg[0] && starts_with(arg[0], "--")) {
if (!strcmp(arg[0], "--url"))
output_url = 1;
if (!strcmp(arg[0], "--name"))