{"thread":{"id":"50202","subject":"[PATCH] remote: check config validity before creating rewrite struct","startedAt":"2019-01-11T19:30:15Z","lastAt":"2019-01-12T02:36:50Z","messageCount":2,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"366571","messageId":"20190111193011.GA19839@sigill.intra.peff.net","threadId":"50202","inReplyTo":null,"subject":"[PATCH] remote: check config validity before creating rewrite struct","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-01-11T19:30:11Z","receivedAt":"2019-01-11T19:30:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When parsing url.foo.insteadOf, we call make_rewrite() and only then\ncheck to make sure the value is a string (and return an error if it\nisn't). This isn't quite a leak, because the struct we allocate is part\nof a global array, but it does leave a funny half-finished struct.\n\nIn practice, it doesn't make much difference because we exit soon after\ndue to the config error anyway. But let's flip the order here to avoid\nleaving a trap for somebody in the future.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis was a minor cleanup I mentioned in:\n\n  https://public-inbox.org/git/20181122173109.GI28192@sigill.intra.peff.net/\n\nbut didn't get picked up.\n\n remote.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 670dd44813..9cc3b07d21 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -337,14 +337,14 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (!name)\n \t\t\treturn 0;\n \t\tif (!strcmp(subkey, \"insteadof\")) {\n-\t\t\trewrite = make_rewrite(&rewrites, name, namelen);\n \t\t\tif (!value)\n \t\t\t\treturn config_error_nonbool(key);\n+\t\t\trewrite = make_rewrite(&rewrites, name, namelen);\n \t\t\tadd_instead_of(rewrite, xstrdup(value));\n \t\t} else if (!strcmp(subkey, \"pushinsteadof\")) {\n-\t\t\trewrite = make_rewrite(&rewrites_push, name, namelen);\n \t\t\tif (!value)\n \t\t\t\treturn config_error_nonbool(key);\n+\t\t\trewrite = make_rewrite(&rewrites_push, name, namelen);\n \t\t\tadd_instead_of(rewrite, xstrdup(value));\n \t\t}\n \t}\n-- \n2.20.1.651.g2d41a78c67\n"},{"id":"366608","messageId":"xmqq5zuuy53a.fsf@gitster-ct.c.googlers.com","threadId":"50202","inReplyTo":"20190111193011.GA19839@sigill.intra.peff.net","subject":"Re: [PATCH] remote: check config validity before creating rewrite struct","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-12T02:36:41Z","receivedAt":"2019-01-12T02:36:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This was a minor cleanup I mentioned in:\n>\n>   https://public-inbox.org/git/20181122173109.GI28192@sigill.intra.peff.net/\n>\n> but didn't get picked up.\n\nYeah, that does sound vaguely familiar ;-)  Thanks, will queue.\n\n>\n>  remote.c | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/remote.c b/remote.c\n> index 670dd44813..9cc3b07d21 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -337,14 +337,14 @@ static int handle_config(const char *key, const char *value, void *cb)\n>  \t\tif (!name)\n>  \t\t\treturn 0;\n>  \t\tif (!strcmp(subkey, \"insteadof\")) {\n> -\t\t\trewrite = make_rewrite(&rewrites, name, namelen);\n>  \t\t\tif (!value)\n>  \t\t\t\treturn config_error_nonbool(key);\n> +\t\t\trewrite = make_rewrite(&rewrites, name, namelen);\n>  \t\t\tadd_instead_of(rewrite, xstrdup(value));\n>  \t\t} else if (!strcmp(subkey, \"pushinsteadof\")) {\n> -\t\t\trewrite = make_rewrite(&rewrites_push, name, namelen);\n>  \t\t\tif (!value)\n>  \t\t\t\treturn config_error_nonbool(key);\n> +\t\t\trewrite = make_rewrite(&rewrites_push, name, namelen);\n>  \t\t\tadd_instead_of(rewrite, xstrdup(value));\n>  \t\t}\n>  \t}\n"}]}