{"thread":{"id":"13015","subject":"[PATCH] builtin-apply.c: use git_config_string() to get apply_default_whitespace","startedAt":"2008-04-08T08:42:33Z","lastAt":"2008-04-15T04:19:34Z","messageCount":4,"participants":["Stephan Beyer","Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"73842","messageId":"20080408084233.GA7785@leksak.fem-net","threadId":"13015","inReplyTo":null,"subject":"[PATCH] builtin-apply.c: use git_config_string() to get apply_default_whitespace","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-04-08T08:42:33Z","receivedAt":"2008-04-08T08:42:33Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Signed-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\nHi,\n\na simple `Janitor patch'.\n\nRegards,\n Stephan.\n\n builtin-apply.c |    8 ++------\n 1 files changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex b5f78ac..ce0a0c3 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -2978,12 +2978,8 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)\n \n static int git_apply_config(const char *var, const char *value)\n {\n-\tif (!strcmp(var, \"apply.whitespace\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tapply_default_whitespace = xstrdup(value);\n-\t\treturn 0;\n-\t}\n+\tif (!strcmp(var, \"apply.whitespace\"))\n+\t\treturn git_config_string(&apply_default_whitespace, var, value);\n \treturn git_default_config(var, value);\n }\n \n-- \n1.5.5.rc3.8.g78d1a\n"},{"id":"74236","messageId":"200804130556.45141.chriscool@tuxfamily.org","threadId":"13015","inReplyTo":"20080408084233.GA7785@leksak.fem-net","subject":"Re: [PATCH] builtin-apply.c: use git_config_string() to get apply_default_whitespace","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-04-13T03:56:44Z","receivedAt":"2008-04-13T03:56:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le mardi 8 avril 2008, Stephan Beyer a écrit :\n> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>\n> ---\n> Hi,\n>\n> a simple `Janitor patch'.\n\nThanks.\n\n>\n> Regards,\n>  Stephan.\n>\n>  builtin-apply.c |    8 ++------\n>  1 files changed, 2 insertions(+), 6 deletions(-)\n>\n> diff --git a/builtin-apply.c b/builtin-apply.c\n> index b5f78ac..ce0a0c3 100644\n> --- a/builtin-apply.c\n> +++ b/builtin-apply.c\n> @@ -2978,12 +2978,8 @@ static int apply_patch(int fd, const char\n> *filename, int inaccurate_eof)\n>\n>  static int git_apply_config(const char *var, const char *value)\n>  {\n> -\tif (!strcmp(var, \"apply.whitespace\")) {\n> -\t\tif (!value)\n> -\t\t\treturn config_error_nonbool(var);\n> -\t\tapply_default_whitespace = xstrdup(value);\n> -\t\treturn 0;\n> -\t}\n> +\tif (!strcmp(var, \"apply.whitespace\"))\n> +\t\treturn git_config_string(&apply_default_whitespace, var, value);\n>  \treturn git_default_config(var, value);\n>  }\n>\n> -- \n> 1.5.5.rc3.8.g78d1a\n\nTested-by: Christian Couder <chriscool@tuxfamily.org>\n\nJunio, please apply.\n\nThanks,\nChristian.\n"},{"id":"74240","messageId":"7vskxqe0db.fsf@gitster.siamese.dyndns.org","threadId":"13015","inReplyTo":"200804130556.45141.chriscool@tuxfamily.org","subject":"Re* [PATCH] builtin-apply.c: use git_config_string() to get apply_default_whitespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-13T06:26:40Z","receivedAt":"2008-04-13T06:26:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> Le mardi 8 avril 2008, Stephan Beyer a écrit :\n>> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>\n>> ---\n>> Hi,\n>>\n>> a simple `Janitor patch'.\n>\n> Thanks.\n> ...\n> Tested-by: Christian Couder <chriscool@tuxfamily.org>\n>\n> Junio, please apply.\n\nHmmmm.\n\n$ git grep -A1 config_error_nonbool | grep -B1 '^[^ ]*-.* = xstrdup'\n\nshows these 13:\n\nalias.c:\t\t\treturn config_error_nonbool(k);\nalias.c-\t\talias_val = xstrdup(v);\n--\nbuiltin-apply.c:\t\t\treturn config_error_nonbool(var);\nbuiltin-apply.c-\t\tapply_default_whitespace = xstrdup(value);\n--\nbuiltin-commit.c:\t\t\treturn config_error_nonbool(v);\nbuiltin-commit.c-\t\ttemplate_file = xstrdup(v);\n--\nbuiltin-log.c:\t\t\tconfig_error_nonbool(var);\nbuiltin-log.c-\t\tfmt_patch_subject_prefix = xstrdup(value);\n--\nbuiltin-log.c:\t\t\treturn config_error_nonbool(var);\nbuiltin-log.c-\t\tfmt_patch_suffix = xstrdup(value);\n--\nconfig.c:\t\treturn config_error_nonbool(var);\nconfig.c-\t*dest = xstrdup(value);\n--\ndiff.c:\t\t\treturn config_error_nonbool(var);\ndiff.c-\t\texternal_diff_cmd_cfg = xstrdup(value);\n--\nhttp.c:\t\t\t\treturn config_error_nonbool(var);\nhttp.c-\t\t\tssl_cert = xstrdup(value);\n--\nhttp.c:\t\t\t\treturn config_error_nonbool(var);\nhttp.c-\t\t\tssl_key = xstrdup(value);\n--\nhttp.c:\t\t\t\treturn config_error_nonbool(var);\nhttp.c-\t\t\tssl_capath = xstrdup(value);\n--\nhttp.c:\t\t\t\treturn config_error_nonbool(var);\nhttp.c-\t\t\tssl_cainfo = xstrdup(value);\n--\nhttp.c:\t\t\t\treturn config_error_nonbool(var);\nhttp.c-\t\t\tcurl_http_proxy = xstrdup(value);\n--\nremote.c:\t\t\t\treturn config_error_nonbool(key);\nremote.c-\t\t\tbranch->remote_name = xstrdup(value);\n\nAmong these, obviously the one in config.c cannot be replaced (it is the\nimplementation of git_config_string() itself ;-), and the one in remote.c\nhas a different pattern (it does not return immediately after that, but\neverything else can be mechanically replaced.\n\nI also have strong doubt about the way http.c handles duplicated\nconfiguration values; everybody else let's the later one override the\nprevious one, while these take and keep the first occurrence.  I left the\nlogic of them as they were, because changing them would change the\nsemantics of the program.\n\n--\n alias.c          |    8 ++------\n builtin-apply.c  |    8 ++------\n builtin-commit.c |    9 ++-------\n builtin-log.c    |   16 ++++------------\n diff.c           |    8 ++------\n http.c           |   44 +++++++++++++++-----------------------------\n 6 files changed, 27 insertions(+), 66 deletions(-)\n\ndiff --git a/alias.c b/alias.c\nindex 116cac8..1513681 100644\n--- a/alias.c\n+++ b/alias.c\n@@ -4,12 +4,8 @@ static const char *alias_key;\n static char *alias_val;\n static int alias_lookup_cb(const char *k, const char *v)\n {\n-\tif (!prefixcmp(k, \"alias.\") && !strcmp(k+6, alias_key)) {\n-\t\tif (!v)\n-\t\t\treturn config_error_nonbool(k);\n-\t\talias_val = xstrdup(v);\n-\t\treturn 0;\n-\t}\n+\tif (!prefixcmp(k, \"alias.\") && !strcmp(k+6, alias_key))\n+\t\treturn git_config_string((const char **)&alias_val, k, v);\n \treturn 0;\n }\n \ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex abe73a0..c8ca41b 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -2981,12 +2981,8 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)\n \n static int git_apply_config(const char *var, const char *value)\n {\n-\tif (!strcmp(var, \"apply.whitespace\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tapply_default_whitespace = xstrdup(value);\n-\t\treturn 0;\n-\t}\n+\tif (!strcmp(var, \"apply.whitespace\"))\n+\t\treturn git_config_string(&apply_default_whitespace, var, value);\n \treturn git_default_config(var, value);\n }\n \ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex bcb7aaa..995fd73 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -827,13 +827,8 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \n int git_commit_config(const char *k, const char *v)\n {\n-\tif (!strcmp(k, \"commit.template\")) {\n-\t\tif (!v)\n-\t\t\treturn config_error_nonbool(v);\n-\t\ttemplate_file = xstrdup(v);\n-\t\treturn 0;\n-\t}\n-\n+\tif (!strcmp(k, \"commit.template\"))\n+\t\treturn git_config_string((const char **)&template_file, k, v);\n \treturn git_status_config(k, v);\n }\n \ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5c00725..59d8f2e 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -226,12 +226,8 @@ static int git_log_config(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"format.pretty\"))\n \t\treturn git_config_string(&fmt_pretty, var, value);\n-\tif (!strcmp(var, \"format.subjectprefix\")) {\n-\t\tif (!value)\n-\t\t\tconfig_error_nonbool(var);\n-\t\tfmt_patch_subject_prefix = xstrdup(value);\n-\t\treturn 0;\n-\t}\n+\tif (!strcmp(var, \"format.subjectprefix\"))\n+\t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n \tif (!strcmp(var, \"log.showroot\")) {\n \t\tdefault_show_root = git_config_bool(var, value);\n \t\treturn 0;\n@@ -478,12 +474,8 @@ static int git_format_config(const char *var, const char *value)\n \t\tadd_header(value);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(var, \"format.suffix\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tfmt_patch_suffix = xstrdup(value);\n-\t\treturn 0;\n-\t}\n+\tif (!strcmp(var, \"format.suffix\"))\n+\t\treturn git_config_string(&fmt_patch_suffix, var, value);\n \tif (!strcmp(var, \"diff.color\") || !strcmp(var, \"color.diff\")) {\n \t\treturn 0;\n \t}\ndiff --git a/diff.c b/diff.c\nindex 8022e67..ce28bbc 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -153,12 +153,8 @@ int git_diff_ui_config(const char *var, const char *value)\n \t\tdiff_auto_refresh_index = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(var, \"diff.external\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\texternal_diff_cmd_cfg = xstrdup(value);\n-\t\treturn 0;\n-\t}\n+\tif (!strcmp(var, \"diff.external\"))\n+\t\treturn git_config_string(&external_diff_cmd_cfg, var, value);\n \tif (!prefixcmp(var, \"diff.\")) {\n \t\tconst char *ep = strrchr(var, '.');\n \ndiff --git a/http.c b/http.c\nindex 256a5f1..9bdf27e 100644\n--- a/http.c\n+++ b/http.c\n@@ -13,18 +13,18 @@ static CURL *curl_default;\n char curl_errorstr[CURL_ERROR_SIZE];\n \n static int curl_ssl_verify = -1;\n-static char *ssl_cert = NULL;\n+static const char *ssl_cert = NULL;\n #if LIBCURL_VERSION_NUM >= 0x070902\n-static char *ssl_key = NULL;\n+static const char *ssl_key = NULL;\n #endif\n #if LIBCURL_VERSION_NUM >= 0x070908\n-static char *ssl_capath = NULL;\n+static const char *ssl_capath = NULL;\n #endif\n-static char *ssl_cainfo = NULL;\n+static const char *ssl_cainfo = NULL;\n static long curl_low_speed_limit = -1;\n static long curl_low_speed_time = -1;\n static int curl_ftp_no_epsv = 0;\n-static char *curl_http_proxy = NULL;\n+static const char *curl_http_proxy = NULL;\n \n static struct curl_slist *pragma_header;\n \n@@ -100,38 +100,27 @@ static int http_options(const char *var, const char *value)\n \t}\n \n \tif (!strcmp(\"http.sslcert\", var)) {\n-\t\tif (ssl_cert == NULL) {\n-\t\t\tif (!value)\n-\t\t\t\treturn config_error_nonbool(var);\n-\t\t\tssl_cert = xstrdup(value);\n-\t\t}\n+\t\tif (ssl_cert == NULL)\n+\t\t\treturn git_config_string(&ssl_cert, var, value);\n \t\treturn 0;\n \t}\n #if LIBCURL_VERSION_NUM >= 0x070902\n \tif (!strcmp(\"http.sslkey\", var)) {\n-\t\tif (ssl_key == NULL) {\n-\t\t\tif (!value)\n-\t\t\t\treturn config_error_nonbool(var);\n-\t\t\tssl_key = xstrdup(value);\n-\t\t}\n+\t\tif (ssl_key == NULL)\n+\t\t\treturn git_config_string(&ssl_key, var, value);\n \t\treturn 0;\n \t}\n #endif\n #if LIBCURL_VERSION_NUM >= 0x070908\n \tif (!strcmp(\"http.sslcapath\", var)) {\n-\t\tif (ssl_capath == NULL) {\n-\t\t\tif (!value)\n-\t\t\t\treturn config_error_nonbool(var);\n-\t\t\tssl_capath = xstrdup(value);\n-\t\t}\n+\t\tif (ssl_capath == NULL)\n+\t\t\treturn git_config_string(&ssl_capath, var, value);\n \t\treturn 0;\n \t}\n #endif\n \tif (!strcmp(\"http.sslcainfo\", var)) {\n \t\tif (ssl_cainfo == NULL) {\n-\t\t\tif (!value)\n-\t\t\t\treturn config_error_nonbool(var);\n-\t\t\tssl_cainfo = xstrdup(value);\n+\t\t\treturn git_config_string(&ssl_cainfo, var, value);\n \t\t}\n \t\treturn 0;\n \t}\n@@ -160,11 +149,8 @@ static int http_options(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \tif (!strcmp(\"http.proxy\", var)) {\n-\t\tif (curl_http_proxy == NULL) {\n-\t\t\tif (!value)\n-\t\t\t\treturn config_error_nonbool(var);\n-\t\t\tcurl_http_proxy = xstrdup(value);\n-\t\t}\n+\t\tif (curl_http_proxy == NULL)\n+\t\t\treturn git_config_string(&curl_http_proxy, var, value);\n \t\treturn 0;\n \t}\n \n@@ -311,7 +297,7 @@ void http_cleanup(void)\n \tpragma_header = NULL;\n \n \tif (curl_http_proxy) {\n-\t\tfree(curl_http_proxy);\n+\t\tfree((char *)curl_http_proxy);\n \t\tcurl_http_proxy = NULL;\n \t}\n }\n"},{"id":"74412","messageId":"200804150619.34775.chriscool@tuxfamily.org","threadId":"13015","inReplyTo":"7vskxqe0db.fsf@gitster.siamese.dyndns.org","subject":"Re: Re* [PATCH] builtin-apply.c: use git_config_string() to get apply_default_whitespace","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-04-15T04:19:34Z","receivedAt":"2008-04-15T04:19:34Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le dimanche 13 avril 2008, Junio C Hamano a écrit :\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> > Le mardi 8 avril 2008, Stephan Beyer a écrit :\n> >> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>\n> >> ---\n> >> Hi,\n> >>\n> >> a simple `Janitor patch'.\n> >\n> > Thanks.\n> > ...\n> > Tested-by: Christian Couder <chriscool@tuxfamily.org>\n> >\n> > Junio, please apply.\n>\n> Hmmmm.\n>\n> $ git grep -A1 config_error_nonbool | grep -B1 '^[^ ]*-.* = xstrdup'\n>\n> shows these 13:\n\nYes, there are many potential places.\n\n> Among these, obviously the one in config.c cannot be replaced (it is the\n> implementation of git_config_string() itself ;-), and the one in remote.c\n> has a different pattern (it does not return immediately after that, but\n> everything else can be mechanically replaced.\n\nI think that each case should be looked at closely. But as it's not very \ndifficult either in many cases, I think it's a good thing to let newbies \ntry their skills at it. That's why I set up the Janitor page on the wiki:\n\nhttp://git.or.cz/gitwiki/Janitor\n\ndescribing this task.\n\n>\n> diff --git a/alias.c b/alias.c\n> index 116cac8..1513681 100644\n> --- a/alias.c\n> +++ b/alias.c\n> @@ -4,12 +4,8 @@ static const char *alias_key;\n>  static char *alias_val;\n>  static int alias_lookup_cb(const char *k, const char *v)\n>  {\n> -\tif (!prefixcmp(k, \"alias.\") && !strcmp(k+6, alias_key)) {\n> -\t\tif (!v)\n> -\t\t\treturn config_error_nonbool(k);\n> -\t\talias_val = xstrdup(v);\n> -\t\treturn 0;\n> -\t}\n> +\tif (!prefixcmp(k, \"alias.\") && !strcmp(k+6, alias_key))\n> +\t\treturn git_config_string((const char **)&alias_val, k, v);\n>  \treturn 0;\n>  }\n\nThat's also the patch that Stephan sent to the list before, see:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/78845\n\nAnd I told him that casting to (const char **) was ugly, and it was better \nto leave this file as it is.\n\n> diff --git a/builtin-apply.c b/builtin-apply.c\n> index abe73a0..c8ca41b 100644\n> --- a/builtin-apply.c\n> +++ b/builtin-apply.c\n> @@ -2981,12 +2981,8 @@ static int apply_patch(int fd, const char\n> *filename, int inaccurate_eof)\n>\n>  static int git_apply_config(const char *var, const char *value)\n>  {\n> -\tif (!strcmp(var, \"apply.whitespace\")) {\n> -\t\tif (!value)\n> -\t\t\treturn config_error_nonbool(var);\n> -\t\tapply_default_whitespace = xstrdup(value);\n> -\t\treturn 0;\n> -\t}\n> +\tif (!strcmp(var, \"apply.whitespace\"))\n> +\t\treturn git_config_string(&apply_default_whitespace, var, value);\n>  \treturn git_default_config(var, value);\n>  }\n\nThat's the next patch Stephan sent, and it is indeed good, so please apply \nStephan's. Thanks.\n\nAbout other files, I don't care much. That's also why I left it to Janitors.\n\nIf I see that you fixed everything I will just remove the Janitor page on \nthe wiki.\n\nRegards,\nChristian.\n"}]}