From 28fc3a6857a5d7a6b4f63b2672fb0ce966b0df78 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 9 Jun 2011 11:51:22 -0400 Subject: strbuf_split: add a max parameter Sometimes when splitting, you only want a limited number of fields, and for the final field to contain "everything else", even if it includes the delimiter. This patch introduces strbuf_split_max, which provides a "max number of fields" parameter; it behaves similarly to perl's "split" with a 3rd field. The existing 2-argument form of strbuf_split is retained for compatibility and ease-of-use. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- strbuf.c | 7 +++++-- strbuf.h | 7 ++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/strbuf.c b/strbuf.c index 77444a94df..74f661ec2b 100644 --- a/strbuf.c +++ b/strbuf.c @@ -101,7 +101,7 @@ void strbuf_ltrim(struct strbuf *sb) sb->buf[sb->len] = '\0'; } -struct strbuf **strbuf_split(const struct strbuf *sb, int delim) +struct strbuf **strbuf_split_max(const struct strbuf *sb, int delim, int max) { int alloc = 2, pos = 0; char *n, *p; @@ -112,7 +112,10 @@ struct strbuf **strbuf_split(const struct strbuf *sb, int delim) p = n = sb->buf; while (n < sb->buf + sb->len) { int len; - n = memchr(n, delim, sb->len - (n - sb->buf)); + if (max <= 0 || pos + 1 < max) + n = memchr(n, delim, sb->len - (n - sb->buf)); + else + n = NULL; if (pos + 1 >= alloc) { alloc = alloc * 2; ret = xrealloc(ret, sizeof(struct strbuf *) * alloc); diff --git a/strbuf.h b/strbuf.h index 07060ce893..5278d64e90 100644 --- a/strbuf.h +++ b/strbuf.h @@ -47,7 +47,12 @@ extern void strbuf_rtrim(struct strbuf *); extern void strbuf_ltrim(struct strbuf *); extern int strbuf_cmp(const struct strbuf *, const struct strbuf *); -extern struct strbuf **strbuf_split(const struct strbuf *, int delim); +extern struct strbuf **strbuf_split_max(const struct strbuf *, + int delim, int max); +static inline struct strbuf **strbuf_split(const struct strbuf *sb, int delim) +{ + return strbuf_split_max(sb, delim, 0); +} extern void strbuf_list_free(struct strbuf **); /*----- add data in your buffer -----*/ -- cgit v1.2.3 From 5bf6529aaa3fa829328ae00ddf7aa851935443b5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 9 Jun 2011 11:51:36 -0400 Subject: fix "git -c" parsing of values with equals signs If you do something like: git -c core.foo="value with = in it" ... we would split your option on "=" into three fields and throw away the third one. With this patch we correctly take everything after the first "=" as the value (keys cannot have an equals sign in them, so the parsing is unambiguous). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 2 +- t/t1300-repo-config.sh | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 61de4d6a17..84ff346859 100644 --- a/config.c +++ b/config.c @@ -45,7 +45,7 @@ static int git_config_parse_parameter(const char *text, struct strbuf tmp = STRBUF_INIT; struct strbuf **pair; strbuf_addstr(&tmp, text); - pair = strbuf_split(&tmp, '='); + pair = strbuf_split_max(&tmp, '=', 2); if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=') strbuf_setlen(pair[0], pair[0]->len - 1); strbuf_trim(pair[0]); diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 3db56267ee..ca5058e0d4 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -904,4 +904,10 @@ test_expect_success 'git -c works with aliases of builtins' ' test_cmp expect actual ' +test_expect_success 'git -c does not split values on equals' ' + echo "value with = in it" >expect && + git -c core.foo="value with = in it" config core.foo >actual && + test_cmp expect actual +' + test_done -- cgit v1.2.3 From 1c2c9bee1bad9a4133a934d04c32b033cc16c8aa Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 9 Jun 2011 11:52:32 -0400 Subject: config: die on error in command-line config The error handling for git_config is somewhat confusing. We collect errors from running git_config_from_file on the various config files and carefully pass them back up. But the two odd things are: 1. We actually die on most errors in git_config_from_file. In fact, the only error we actually pass back up is if fopen() fails on the file. 2. Most callers of git_config do not check the error return at all, but will continue if git_config reports an error. When the code for "git -c core.foo=bar" was added, it dutifully passed errors up the call stack, only for them to be eventually ignored. This makes it inconsistent with the file-parsing code, which will die when it sees malformed config. And it's somewhat unsafe, because it means an error in parsing a typo like: git -c clean.requireforce=ture clean will continue the command, ignoring the config the user tried to give. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 2 +- t/t1300-repo-config.sh | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 84ff346859..2567b546a3 100644 --- a/config.c +++ b/config.c @@ -858,7 +858,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) switch (git_config_from_parameters(fn, data)) { case -1: /* error */ - ret--; + die("unable to parse command-line config"); break; case 0: /* found nothing */ break; diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index ca5058e0d4..584e956ac5 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -910,4 +910,12 @@ test_expect_success 'git -c does not split values on equals' ' test_cmp expect actual ' +test_expect_success 'git -c dies on bogus config' ' + test_must_fail git -c core.bare=foo rev-parse +' + +test_expect_success 'git -c complains about empty key' ' + test_must_fail git -c "=foo" rev-parse +' + test_done -- cgit v1.2.3 From c5d6350bdc8d0d8bd4bd1aa0273313e71cd548f6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 9 Jun 2011 11:52:43 -0400 Subject: config: avoid segfault when parsing command-line config We already check for an empty key on the left side of an equals, but we would segfault if there was no content at all. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 2 ++ t/t1300-repo-config.sh | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/config.c b/config.c index 2567b546a3..9939f65d9e 100644 --- a/config.c +++ b/config.c @@ -46,6 +46,8 @@ static int git_config_parse_parameter(const char *text, struct strbuf **pair; strbuf_addstr(&tmp, text); pair = strbuf_split_max(&tmp, '=', 2); + if (!pair[0]) + return error("bogus config parameter: %s", text); if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=') strbuf_setlen(pair[0], pair[0]->len - 1); strbuf_trim(pair[0]); diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 584e956ac5..3e140c18f4 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -918,4 +918,8 @@ test_expect_success 'git -c complains about empty key' ' test_must_fail git -c "=foo" rev-parse ' +test_expect_success 'git -c complains about empty key and value' ' + test_must_fail git -c "" rev-parse +' + test_done -- cgit v1.2.3 From 2f1d9e2b93e1b7fbfcfa59331db89dd6c76a3505 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 9 Jun 2011 11:54:58 -0400 Subject: strbuf: allow strbuf_split to work on non-strbufs The strbuf_split function takes a strbuf as input, and outputs a list of strbufs. However, there is no reason that the input has to be a strbuf, and not an arbitrary buffer. This patch adds strbuf_split_buf for a length-delimited buffer, and strbuf_split_str for NUL-terminated strings. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- strbuf.c | 12 ++++++------ strbuf.h | 12 +++++++++++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/strbuf.c b/strbuf.c index 74f661ec2b..a71de9cd63 100644 --- a/strbuf.c +++ b/strbuf.c @@ -101,19 +101,19 @@ void strbuf_ltrim(struct strbuf *sb) sb->buf[sb->len] = '\0'; } -struct strbuf **strbuf_split_max(const struct strbuf *sb, int delim, int max) +struct strbuf **strbuf_split_buf(const char *str, size_t slen, int delim, int max) { int alloc = 2, pos = 0; - char *n, *p; + const char *n, *p; struct strbuf **ret; struct strbuf *t; ret = xcalloc(alloc, sizeof(struct strbuf *)); - p = n = sb->buf; - while (n < sb->buf + sb->len) { + p = n = str; + while (n < str + slen) { int len; if (max <= 0 || pos + 1 < max) - n = memchr(n, delim, sb->len - (n - sb->buf)); + n = memchr(n, delim, slen - (n - str)); else n = NULL; if (pos + 1 >= alloc) { @@ -121,7 +121,7 @@ struct strbuf **strbuf_split_max(const struct strbuf *sb, int delim, int max) ret = xrealloc(ret, sizeof(struct strbuf *) * alloc); } if (!n) - n = sb->buf + sb->len - 1; + n = str + slen - 1; len = n - p + 1; t = xmalloc(sizeof(struct strbuf)); strbuf_init(t, len); diff --git a/strbuf.h b/strbuf.h index 5278d64e90..e7e674bf1f 100644 --- a/strbuf.h +++ b/strbuf.h @@ -47,8 +47,18 @@ extern void strbuf_rtrim(struct strbuf *); extern void strbuf_ltrim(struct strbuf *); extern int strbuf_cmp(const struct strbuf *, const struct strbuf *); -extern struct strbuf **strbuf_split_max(const struct strbuf *, +extern struct strbuf **strbuf_split_buf(const char *, size_t, int delim, int max); +static inline struct strbuf **strbuf_split_str(const char *str, + int delim, int max) +{ + return strbuf_split_buf(str, strlen(str), delim, max); +} +static inline struct strbuf **strbuf_split_max(const struct strbuf *sb, + int delim, int max) +{ + return strbuf_split_buf(sb->buf, sb->len, delim, max); +} static inline struct strbuf **strbuf_split(const struct strbuf *sb, int delim) { return strbuf_split_max(sb, delim, 0); -- cgit v1.2.3 From f77bccaeba7a4c542e9b89d144af74bddd36fd08 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 9 Jun 2011 11:55:09 -0400 Subject: config: use strbuf_split_str instead of a temporary strbuf This saves an allocation and copy, and also fixes a minor memory leak. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config.c b/config.c index 9939f65d9e..44b2c93b24 100644 --- a/config.c +++ b/config.c @@ -42,10 +42,8 @@ void git_config_push_parameter(const char *text) static int git_config_parse_parameter(const char *text, config_fn_t fn, void *data) { - struct strbuf tmp = STRBUF_INIT; struct strbuf **pair; - strbuf_addstr(&tmp, text); - pair = strbuf_split_max(&tmp, '=', 2); + pair = strbuf_split_str(text, '=', 2); if (!pair[0]) return error("bogus config parameter: %s", text); if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=') -- cgit v1.2.3