Hi Junio,
On 2015-06-19 22:39, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>
>> + 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);
--
To unsubscribe from this list: send the line "unsubscribe git" in