From 5ab50417b7d51fff1c88158f0f334f42bfb5a9b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 7 Jun 2011 22:49:13 +0200 Subject: [PATCH 1/5] Remove an unfortunate optimisation from cvar_match_section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The (rather late) early-exit code, which provides a negligible optimisation causes cvar_match_section to return false negatives when it's called with a section name instead of a full variable name. Remove this optimisation. Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index d76c6024d..74162d74f 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -130,7 +130,7 @@ static void cvar_list_free(cvar_t_list *list) */ static int cvar_match_section(const char *local, const char *input) { - char *first_dot, *last_dot; + char *first_dot; char *local_sp = strchr(local, ' '); int comparison_len; @@ -159,12 +159,8 @@ static int cvar_match_section(const char *local, const char *input) */ first_dot = strchr(input, '.'); - last_dot = strrchr(input, '.'); comparison_len = strlen(local_sp + 2) - 1; - if (last_dot == first_dot || last_dot - first_dot - 1 != comparison_len) - return 0; - return !strncmp(local_sp + 2, first_dot + 1, comparison_len); } From 3b3577c764ad06cd24bb9047107022abdf6fea90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 7 Jun 2011 23:32:14 +0200 Subject: [PATCH 2/5] config: store new variables with the internal representation of the section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The section name should be stored in its case-sensitive variant when we are adding a new variable. Use the internalize_section function to do just that. Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/config_file.c b/src/config_file.c index 74162d74f..5e3dabdf7 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -237,6 +237,39 @@ static int cvar_normalize_name(cvar_t *var, char **output) return GIT_SUCCESS; } +static char *interiorize_section(const char *orig) +{ + char *dot, *last_dot, *section, *ret; + int len; + + dot = strchr(orig, '.'); + last_dot = strrchr(orig, '.'); + len = last_dot - orig; + + /* No subsection, this is easy */ + if (last_dot == dot) + return git__strndup(orig, dot - orig); + + section = git__malloc(len + 4); + if (section == NULL) + return NULL; + + memset(section, 0x0, len + 4); + ret = section; + len = dot - orig; + memcpy(section, orig, len); + section += len; + len = STRLEN(" \""); + memcpy(section, " \"", len); + section += len; + len = last_dot - dot - 1; + memcpy(section, dot + 1, len); + section += len; + *section = '"'; + + return ret; +} + static int config_open(git_config_file *cfg) { int error; @@ -334,7 +367,7 @@ static int config_set(git_config_file *cfg, const char *name, const char *value) memset(var, 0x0, sizeof(cvar_t)); - var->section = git__strndup(name, last_dot - name); + var->section = interiorize_section(name); if (var->section == NULL) { error = GIT_ENOMEM; goto out; From 8bb198e6744daa16cde3f8cea3669b86df83897d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 17 May 2011 16:39:09 +0200 Subject: [PATCH 3/5] config: implement config writing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After each variable gets set, we store it in our list (not completely in the right position, but the close enough). Then we write out the new config file in the same way that git.git does it (keep the rest of the file intact and insert or replace the variable in its line). Overwriting variables and adding new ones is supported (even on new sections), though deleting isn't yet. Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 173 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 172 insertions(+), 1 deletion(-) diff --git a/src/config_file.c b/src/config_file.c index 5e3dabdf7..87a732c78 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -26,9 +26,11 @@ #include "common.h" #include "config.h" #include "fileops.h" +#include "filebuf.h" #include "git2/config.h" #include "git2/types.h" + #include typedef struct cvar_t { @@ -98,6 +100,7 @@ typedef struct { static int config_parse(diskfile_backend *cfg_file); static int parse_variable(diskfile_backend *cfg, char **var_name, char **var_value); +static int config_write(diskfile_backend *cfg, cvar_t *var); static void cvar_free(cvar_t *var) { @@ -349,7 +352,7 @@ static int config_set(git_config_file *cfg, const char *name, const char *value) free(existing->value); existing->value = tmp; - return GIT_SUCCESS; + return config_write(b, existing); } /* @@ -386,6 +389,7 @@ static int config_set(git_config_file *cfg, const char *name, const char *value) } CVAR_LIST_APPEND(&b->var_list, var); + error = config_write(b, var); out: if (error < GIT_SUCCESS) @@ -892,6 +896,173 @@ static int config_parse(diskfile_backend *cfg_file) return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to parse config"); } +static int write_section(git_filebuf *file, cvar_t *var) +{ + int error; + + error = git_filebuf_printf(file, "[%s]\n", var->section); + if (error < GIT_SUCCESS) + return error; + + error = git_filebuf_printf(file, " %s = %s\n", var->name, var->value); + return error; +} + +/* + * This is pretty much the parsing, except we write out anything we don't have + */ +static int config_write(diskfile_backend *cfg, cvar_t *var) +{ + int error = GIT_SUCCESS, c; + int section_matches = 0, last_section_matched = 0; + char *current_section = NULL; + char *var_name, *var_value, *data_start; + git_filebuf file; + const char *pre_end = NULL, *post_start = NULL; + + /* We need to read in our own config file */ + error = gitfo_read_file(&cfg->reader.buffer, cfg->file_path); + if (error < GIT_SUCCESS) { + return git__rethrow(error, "Failed to read existing config file %s", cfg->file_path); + } + + /* Initialise the reading position */ + cfg->reader.read_ptr = cfg->reader.buffer.data; + cfg->reader.eof = 0; + data_start = cfg->reader.read_ptr; + + /* Lock the file */ + error = git_filebuf_open(&file, cfg->file_path, 0); + if (error < GIT_SUCCESS) + return git__rethrow(error, "Failed to lock config file"); + + skip_bom(cfg); + + while (error == GIT_SUCCESS && !cfg->reader.eof) { + c = cfg_peek(cfg, SKIP_WHITESPACE); + + switch (c) { + case '\0': /* We've arrived at the end of the file */ + break; + + case '[': /* section header, new section begins */ + /* + * We set both positions to the current one in case we + * need to add a variable to the end of a section. In that + * case, we want both variables to point just before the + * new section. If we actually want to replace it, the + * default case will take care of updating them. + */ + pre_end = post_start = cfg->reader.read_ptr; + free(current_section); + error = parse_section_header(cfg, ¤t_section); + if (error < GIT_SUCCESS) + break; + + /* Keep track of when it stops matching */ + last_section_matched = section_matches; + section_matches = !strcmp(current_section, var->section); + break; + + case ';': + case '#': + cfg_consume_line(cfg); + break; + + default: + /* + * If the section doesn't match, but the last section did, + * it means we need to add a variable (so skip the line + * otherwise). If both the section and name match, we need + * to overwrite the variable (so skip the line + * otherwise). pre_end needs to be updated each time so we + * don't loose that information, but we only need to + * update post_start if we're going to use it in this + * iteration. + */ + if (!section_matches) { + if (!last_section_matched) { + cfg_consume_line(cfg); + break; + } + } else { + pre_end = cfg->reader.read_ptr; + error = parse_variable(cfg, &var_name, &var_value); + if (error < GIT_SUCCESS || strcasecmp(var->name, var_name)) + break; + post_start = cfg->reader.read_ptr; + } + + /* + * We've found the variable we wanted to change, so + * write anything up to it + */ + error = git_filebuf_write(&file, data_start, pre_end - data_start); + if (error < GIT_SUCCESS) { + git__rethrow(error, "Failed to write the first part of the file"); + break; + } + + /* Then replace the variable */ + error = git_filebuf_printf(&file, " %s = %s\n", var->name, var->value); + if (error < GIT_SUCCESS) { + git__rethrow(error, "Failed to overwrite the variable"); + break; + } + + /* And then the write out rest of the file */ + error = git_filebuf_write(&file, post_start, + cfg->reader.buffer.len - (post_start - data_start)); + + if (error < GIT_SUCCESS) { + git__rethrow(error, "Failed to write the rest of the file"); + break; + } + + goto cleanup; + } + } + + /* + * Being here can mean that + * + * 1) our section is the last one in the file and we're + * adding a variable + * + * 2) we didn't find a section for us so we need to create it + * ourselves. + * + * Either way we need to write out the whole file. + */ + + error = git_filebuf_write(&file, cfg->reader.buffer.data, cfg->reader.buffer.len); + if (error < GIT_SUCCESS) { + git__rethrow(error, "Failed to write original config content"); + goto cleanup; + } + + /* And now if we just need to add a variable */ + if (section_matches) { + error = git_filebuf_printf(&file, " %s = %s\n", var->name, var->value); + goto cleanup; + } + + /* Or maybe we need to write out a whole section */ + error = write_section(&file, var); + if (error < GIT_SUCCESS) + git__rethrow(error, "Failed to write new section"); + + cleanup: + free(current_section); + + if (error < GIT_SUCCESS) + git_filebuf_cleanup(&file); + else + error = git_filebuf_commit(&file); + + return error; +} + static int is_multiline_var(const char *str) { char *end = strrchr(str, '\0') - 1; From 711b1096f361ccdadfd3b316e41ca93ce1757975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 14 Jun 2011 13:08:30 +0200 Subject: [PATCH 4/5] Indent config variables with tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confg variables are indended using tags and not four spaces as was being done by the code. Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index 87a732c78..916b4d081 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -1004,7 +1004,7 @@ static int config_write(diskfile_backend *cfg, cvar_t *var) } /* Then replace the variable */ - error = git_filebuf_printf(&file, " %s = %s\n", var->name, var->value); + error = git_filebuf_printf(&file, "\t%s = %s\n", var->name, var->value); if (error < GIT_SUCCESS) { git__rethrow(error, "Failed to overwrite the variable"); break; @@ -1043,7 +1043,7 @@ static int config_write(diskfile_backend *cfg, cvar_t *var) /* And now if we just need to add a variable */ if (section_matches) { - error = git_filebuf_printf(&file, " %s = %s\n", var->name, var->value); + error = git_filebuf_printf(&file, "\t%s = %s\n", var->name, var->value); goto cleanup; } From a98b0d80dcfdc088419546e856eca43c1c1c1e9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 14 Jun 2011 14:26:08 +0200 Subject: [PATCH 5/5] Test replacing a value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a test to check that value replacement works. Signed-off-by: Carlos Martín Nieto --- tests/resources/config/config9 | Bin 0 -> 18 bytes tests/t15-config.c | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 tests/resources/config/config9 diff --git a/tests/resources/config/config9 b/tests/resources/config/config9 new file mode 100644 index 0000000000000000000000000000000000000000..4359c7826d86b2d7b3b182475612c3e9690fc734 GIT binary patch literal 18 Zcmaz}&M!)h<>E{!&CRV;uvIYR0suK-1z7+9 literal 0 HcmV?d00001 diff --git a/tests/t15-config.c b/tests/t15-config.c index 08a2cdbf2..c11c5a932 100644 --- a/tests/t15-config.c +++ b/tests/t15-config.c @@ -189,6 +189,27 @@ BEGIN_TEST(config8, "don't fail on empty files") git_config_free(cfg); END_TEST +BEGIN_TEST +(config9, "replace a value") + git_config *cfg; + int i; + + /* By freeing the config, we make sure we flush the values */ + must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config9")); + must_pass(git_config_set_int(cfg, "core.dummy", 5)); + git_config_free(cfg); + + must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config9")); + must_pass(git_config_get_int(cfg, "core.dummy", &i)); + must_be_true(i == 5); + git_config_free(cfg); + + must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config9")); + must_pass(git_config_set_int(cfg, "core.dummy", 1)); + git_config_free(cfg); + +END_TEST + BEGIN_SUITE(config) ADD_TEST(config0); ADD_TEST(config1); @@ -199,4 +220,5 @@ BEGIN_SUITE(config) ADD_TEST(config6); ADD_TEST(config7); ADD_TEST(config8); + ADD_TEST(config9); END_SUITE