Re: [PATCH v6 18/19] fsck: git receive-pack: support excluding objects from fsck'ing
From: Johannes Schindelin <hidden>
Date: 2016-06-15 23:05:25
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH v6 18/19] fsck: git receive-pack: support excluding objects from fsck'ing · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH v6 18/19] fsck: git receive-pack: support excluding objects from fsck'ing · Johannes Schindelin <hidden>
- 2016-06-15 · Re: [PATCH v6 18/19] fsck: git receive-pack: support excluding objects from fsck'ing · Junio C Hamano <hidden>
- 2016-06-15 · [PATCH v6 18/19] fsck: git receive-pack: support excluding objects from fsck'ing · Johannes Schindelin <hidden>
Hi Junio, On 2015-06-19 22:39, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:quoted
+ if (strcmp(var, "receive.fsck.skiplist") == 0) { + const char *path = is_absolute_path(value) ? + value : git_path("%s", value);This "either absolute or inside $GIT_DIR" looks somewhat strange to me. Shouldn't we mimick what "git config --path" does instead?
Okay. That would also support user paths. There is a problem, though: `git_config_pathname()` accepts a `const char **` parameter to set the path, yet I need to `free()` that pointer afterwards because it has been obtained through `expand_user_path()` which detaches that buffer from a `strbuf`. In regular use cases, that does not matter much because the path obtained via `git_config_pathname()` is typically stored in a global variable, but in my case, I append it to a global variable. Of course I could cast the value to a `char *` and `free()` it, but that would assume too much about the implementation detail of `git_config_pathname()` for my taste. The easiest cop out would be to recapitulate the code in `git_config_pathname()`, but I think the correct solution would be to change the signature of `git_config_pathname()` to accept a `char **` instead. Do you agree? However, the diff looks really ugly: -- snipsnap --
diff --git a/builtin/commit.c b/builtin/commit.c
index 254477f..946b31b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c@@ -1494,7 +1494,7 @@ static int git_commit_config(const char *k, const char *v, void *cb) int status; if (!strcmp(k, "commit.template")) - return git_config_pathname(&template_file, k, v); + return git_config_pathname((char **)&template_file, k, v); if (!strcmp(k, "commit.status")) { include_status = git_config_bool(k, v); return 0;
diff --git a/builtin/config.c b/builtin/config.c
index 7188405..a249faa 100644
--- a/builtin/config.c
+++ b/builtin/config.c@@ -130,7 +130,7 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value else sprintf(value, "%d", v); } else if (types == TYPE_PATH) { - if (git_config_pathname(&vptr, key_, value_) < 0) + if (git_config_pathname((char **)&vptr, key_, value_) < 0) return -1; must_free_vptr = 1; } else if (value_) {
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 4335738..0572cae 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c@@ -177,7 +177,8 @@ free_return: static int git_init_db_config(const char *k, const char *v, void *cb) { if (!strcmp(k, "init.templatedir")) - return git_config_pathname(&init_db_template_dir, k, v); + return git_config_pathname((char **)&init_db_template_dir, + k, v); return 0; }
diff --git a/builtin/log.c b/builtin/log.c
index 8781049..8751173 100644
--- a/builtin/log.c
+++ b/builtin/log.c@@ -743,7 +743,8 @@ static int git_format_config(const char *var, const char *value, void *cb) if (!strcmp(var, "format.signature")) return git_config_string(&signature, var, value); if (!strcmp(var, "format.signaturefile")) - return git_config_pathname(&signature_file, var, value); + return git_config_pathname((char **)&signature_file, + var, value); if (!strcmp(var, "format.coverletter")) { if (value && !strcasecmp(value, "auto")) { config_cover_letter = COVER_AUTO;
diff --git a/cache.h b/cache.h
index 4f55466..19af52e 100644
--- a/cache.h
+++ b/cache.h@@ -1442,7 +1442,7 @@ extern int git_config_bool_or_int(const char *, const char *, int *); extern int git_config_bool(const char *, const char *); extern int git_config_maybe_bool(const char *, const char *); extern int git_config_string(const char **, const char *, const char *); -extern int git_config_pathname(const char **, const char *, const char *); +extern int git_config_pathname(char **, const char *, const char *); extern int git_config_set_in_file(const char *, const char *, const char *); extern int git_config_set(const char *, const char *); extern int git_config_parse_key(const char *, char **, int *);
diff --git a/config.c b/config.c
index 29fa012..51781b8 100644
--- a/config.c
+++ b/config.c@@ -670,7 +670,7 @@ int git_config_string(const char **dest, const char *var, const char *value) return 0; } -int git_config_pathname(const char **dest, const char *var, const char *value) +int git_config_pathname(char **dest, const char *var, const char *value) { if (!value) return config_error_nonbool(var);
@@ -714,7 +714,8 @@ static int git_default_core_config(const char *var, const char *value) } if (!strcmp(var, "core.attributesfile")) - return git_config_pathname(&git_attributes_file, var, value); + return git_config_pathname((char **)&git_attributes_file, + var, value); if (!strcmp(var, "core.bare")) { is_bare_repository_cfg = git_config_bool(var, value);
@@ -862,7 +863,7 @@ static int git_default_core_config(const char *var, const char *value) return git_config_string(&askpass_program, var, value); if (!strcmp(var, "core.excludesfile")) - return git_config_pathname(&excludes_file, var, value); + return git_config_pathname((char **)&excludes_file, var, value); if (!strcmp(var, "core.whitespace")) { if (!value)
@@ -989,7 +990,8 @@ static int git_default_push_config(const char *var, const char *value) static int git_default_mailmap_config(const char *var, const char *value) { if (!strcmp(var, "mailmap.file")) - return git_config_pathname(&git_mailmap_file, var, value); + return git_config_pathname((char **)&git_mailmap_file, + var, value); if (!strcmp(var, "mailmap.blob")) return git_config_string(&git_mailmap_blob, var, value);
@@ -1506,7 +1508,7 @@ int git_configset_get_pathname(struct config_set *cs, const char *key, const cha { const char *value; if (!git_configset_get_value(cs, key, &value)) - return git_config_pathname(dest, key, value); + return git_config_pathname((char **)dest, key, value); else return 1; }
diff --git a/diff.c b/diff.c
index 87b16d5..e029b75 100644
--- a/diff.c
+++ b/diff.c@@ -204,7 +204,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb) if (!strcmp(var, "diff.wordregex")) return git_config_string(&diff_word_regex_cfg, var, value); if (!strcmp(var, "diff.orderfile")) - return git_config_pathname(&diff_order_file_cfg, var, value); + return git_config_pathname((char **)&diff_order_file_cfg, + var, value); if (!strcmp(var, "diff.ignoresubmodules")) handle_ignore_submodules_arg(&default_diff_options, value);