{"thread":{"id":"40900","subject":"[PATCHv3] ident.c: add support for IPv6","startedAt":"2015-11-27T14:08:27Z","lastAt":"2015-11-28T20:48:27Z","messageCount":3,"participants":["Elia Pinto","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"273796","messageId":"1448633307-43339-1-git-send-email-gitter.spiros@gmail.com","threadId":"40900","inReplyTo":null,"subject":"[PATCHv3] ident.c: add support for IPv6","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-11-27T14:08:27Z","receivedAt":"2015-11-27T14:08:27Z","isPatch":false,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"Add IPv6 support by implementing name resolution with the\nprotocol agnostic getaddrinfo(3) API. The old gethostbyname(3)\ncode is still available when git is compiled with NO_IPV6.\n\nSigned-off-by: Elia Pinto \nHelped-by: Jeff King \nHelped-by: Eric Sunshine \n---\nThis is the third version of the patch ($gmane/280488)\nChanges from previous:\n\n- Simplified the implementation, adding the new\nfunction canonical_name (Jeff King) ($gmane/281479).\nFixed a new typo introduced in the second version.\n\nSigned-off-by: Elia Pinto \n---\n ident.c | 32 +++++++++++++++++++++++++++-----\n 1 file changed, 27 insertions(+), 5 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 5ff1aad..6695537 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -70,10 +70,33 @@ static int add_mailname_host(struct strbuf *buf)\n \treturn 0;\n }\n \n+static int canonical_name(const char *host, struct strbuf *out)\n+{\n+ int status=-1;\n+#ifndef NO_IPV6\n+ struct addrinfo hints, *ai;\n+ memset (&hints, '\\0', sizeof (hints));\n+ hints.ai_flags = AI_CANONNAME;\n+ int gai = getaddrinfo(host, NULL, &hints, &ai);\n+ if (!gai) {\n+ if (ai && strchr(ai->ai_canonname, '.')) {\n+ strbuf_addstr(out, ai->ai_canonname);\n+ status=0;\n+ }\n+ freeaddrinfo(ai);\n+ }\n+#else\n+ struct hostent *he = gethostbyname(buf);\n+ if (he && strchr(he->h_name, '.')) {\n+ strbuf_addstr(out, he->h_name);\n+ status=0;\n+ }\n+#endif /* NO_IPV6 */\n+}\n+\n static void add_domainname(struct strbuf *out)\n {\n \tchar buf[1024];\n-\tstruct hostent *he;\n \n \tif (gethostname(buf, sizeof(buf))) {\n \t\twarning(\"cannot get host name: %s\", strerror(errno));\n@@ -82,10 +105,9 @@ static void add_domainname(struct strbuf *out)\n \t}\n \tif (strchr(buf, '.'))\n \t\tstrbuf_addstr(out, buf);\n-\telse if ((he = gethostbyname(buf)) && strchr(he->h_name, '.'))\n-\t\tstrbuf_addstr(out, he->h_name);\n-\telse\n-\t\tstrbuf_addf(out, \"%s.(none)\", buf);\n+\telse {\n+\t\t if (canonical_name(buf,out) != 0) strbuf_addf(out, \"%s.(none)\", buf);\n+\t}\n }\n \n static void copy_email(const struct passwd *pw, struct strbuf *email)\n-- \n2.5.0\n"},{"id":"273815","messageId":"20151128172341.GE27264@sigill.intra.peff.net","threadId":"40900","inReplyTo":"1448633307-43339-1-git-send-email-gitter.spiros@gmail.com","subject":"Re: [PATCHv3] ident.c: add support for IPv6","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-28T17:23:41Z","receivedAt":"2015-11-28T17:23:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 27, 2015 at 02:08:27PM +0000, Elia Pinto wrote:\n\n> This is the third version of the patch ($gmane/280488)\n> Changes from previous:\n> \n> - Simplified the implementation, adding the new\n> function canonical_name (Jeff King) ($gmane/281479).\n> Fixed a new typo introduced in the second version.\n\nThanks, I think this keeps add_domainname() a lot cleaner. There are a\nfew problems:\n\n> +static int canonical_name(const char *host, struct strbuf *out)\n> +{\n> + int status=-1;\n\nOur style is to put whitespace between operators (so \"int status = -1;\",\nand other places below).\n\nThis line (and the others) was also indented with spaces, not tabs.\n\n> +#ifndef NO_IPV6\n> + struct addrinfo hints, *ai;\n> + memset (&hints, '\\0', sizeof (hints));\n> + hints.ai_flags = AI_CANONNAME;\n> + int gai = getaddrinfo(host, NULL, &hints, &ai);\n\nWe do C89-style no-decl-after-statement. But this \"gai\" is only\nused once, so we can just lump it into the next conditional.\n\n> + if (!gai) {\n> + if (ai && strchr(ai->ai_canonname, '.')) {\n> + strbuf_addstr(out, ai->ai_canonname);\n> + status=0;\n> + }\n> + freeaddrinfo(ai);\n> + }\n> +#else\n> + struct hostent *he = gethostbyname(buf);\n> + if (he && strchr(he->h_name, '.')) {\n> + strbuf_addstr(out, he->h_name);\n> + status=0;\n> + }\n> +#endif /* NO_IPV6 */\n> +}\n\nI think you are missing a \"return status\" here.\n\n> @@ -82,10 +105,9 @@ static void add_domainname(struct strbuf *out)\n> \t}\n> \tif (strchr(buf, '.'))\n> \t\tstrbuf_addstr(out, buf);\n> -\telse if ((he = gethostbyname(buf)) && strchr(he->h_name, '.'))\n> -\t\tstrbuf_addstr(out, he->h_name);\n> -\telse\n> -\t\tstrbuf_addf(out, \"%s.(none)\", buf);\n> +\telse {\n> +\t\t if (canonical_name(buf,out) != 0) strbuf_addf(out, \"%s.(none)\", buf);\n> +\t}\n\nWe always put conditional bodies on the next line, even if they are\none-liners.\n\nHere's what I've queued, addressing those. No need to re-send if you're\nhappy with it:\n\n-- >8 --\nFrom: Elia Pinto \nSubject: [PATCH] ident.c: add support for IPv6\n\nAdd IPv6 support by implementing name resolution with the\nprotocol agnostic getaddrinfo(3) API. The old gethostbyname(3)\ncode is still available when git is compiled with NO_IPV6.\n\nSigned-off-by: Elia Pinto \nHelped-by: Jeff King \nHelped-by: Eric Sunshine \nSigned-off-by: Jeff King \n---\n ident.c | 31 +++++++++++++++++++++++++++----\n 1 file changed, 27 insertions(+), 4 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 5ff1aad..4e7f99d 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -70,10 +70,35 @@ static int add_mailname_host(struct strbuf *buf)\n \treturn 0;\n }\n \n+static int canonical_name(const char *host, struct strbuf *out)\n+{\n+\tint status = -1;\n+\n+#ifndef NO_IPV6\n+\tstruct addrinfo hints, *ai;\n+\tmemset (&hints, '\\0', sizeof (hints));\n+\thints.ai_flags = AI_CANONNAME;\n+\tif (!getaddrinfo(host, NULL, &hints, &ai)) {\n+\t\tif (ai && strchr(ai->ai_canonname, '.')) {\n+\t\t\tstrbuf_addstr(out, ai->ai_canonname);\n+\t\t\tstatus = 0;\n+\t\t}\n+\t\tfreeaddrinfo(ai);\n+\t}\n+#else\n+\tstruct hostent *he = gethostbyname(buf);\n+\tif (he && strchr(he->h_name, '.')) {\n+\t\tstrbuf_addstr(out, he->h_name);\n+\t\tstatus = 0;\n+\t}\n+#endif /* NO_IPV6 */\n+\n+\treturn status;\n+}\n+\n static void add_domainname(struct strbuf *out)\n {\n \tchar buf[1024];\n-\tstruct hostent *he;\n \n \tif (gethostname(buf, sizeof(buf))) {\n \t\twarning(\"cannot get host name: %s\", strerror(errno));\n@@ -82,9 +107,7 @@ static void add_domainname(struct strbuf *out)\n \t}\n \tif (strchr(buf, '.'))\n \t\tstrbuf_addstr(out, buf);\n-\telse if ((he = gethostbyname(buf)) && strchr(he->h_name, '.'))\n-\t\tstrbuf_addstr(out, he->h_name);\n-\telse\n+\telse if (canonical_name(buf, out) < 0)\n \t\tstrbuf_addf(out, \"%s.(none)\", buf);\n }\n \n-- \n2.6.3.636.g1460207\n"},{"id":"273817","messageId":"CA+EOSB=i3973MAj1Ti4tbQY23Hq=SDy8zoFLJQ4r1xm7hmZACQ@mail.gmail.com","threadId":"40900","inReplyTo":"20151128172341.GE27264@sigill.intra.peff.net","subject":"Re: [PATCHv3] ident.c: add support for IPv6","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-11-28T20:48:27Z","receivedAt":"2015-11-28T20:48:27Z","isPatch":false,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"2015-11-28 18:23 GMT+01:00, Jeff King :\n> On Fri, Nov 27, 2015 at 02:08:27PM +0000, Elia Pinto wrote:\n>\n>> This is the third version of the patch ($gmane/280488)\n>> Changes from previous:\n>>\n>> - Simplified the implementation, adding the new\n>> function canonical_name (Jeff King) ($gmane/281479).\n>> Fixed a new typo introduced in the second version.\n>\n> Thanks, I think this keeps add_domainname() a lot cleaner. There are a\n> few problems:\n>\n>> +static int canonical_name(const char *host, struct strbuf *out)\n>> +{\n>> + int status=-1;\n>\n> Our style is to put whitespace between operators (so \"int status = -1;\",\n> and other places below).\n>\n> This line (and the others) was also indented with spaces, not tabs.\n>\n>> +#ifndef NO_IPV6\n>> + struct addrinfo hints, *ai;\n>> + memset (&hints, '\\0', sizeof (hints));\n>> + hints.ai_flags = AI_CANONNAME;\n>> + int gai = getaddrinfo(host, NULL, &hints, &ai);\n>\n> We do C89-style no-decl-after-statement. But this \"gai\" is only\n> used once, so we can just lump it into the next conditional.\n>\n>> + if (!gai) {\n>> + if (ai && strchr(ai->ai_canonname, '.')) {\n>> + strbuf_addstr(out, ai->ai_canonname);\n>> + status=0;\n>> + }\n>> + freeaddrinfo(ai);\n>> + }\n>> +#else\n>> + struct hostent *he = gethostbyname(buf);\n>> + if (he && strchr(he->h_name, '.')) {\n>> + strbuf_addstr(out, he->h_name);\n>> + status=0;\n>> + }\n>> +#endif /* NO_IPV6 */\n>> +}\n>\n> I think you are missing a \"return status\" here.\n>\n>> @@ -82,10 +105,9 @@ static void add_domainname(struct strbuf *out)\n>> \t}\n>> \tif (strchr(buf, '.'))\n>> \t\tstrbuf_addstr(out, buf);\n>> -\telse if ((he = gethostbyname(buf)) && strchr(he->h_name, '.'))\n>> -\t\tstrbuf_addstr(out, he->h_name);\n>> -\telse\n>> -\t\tstrbuf_addf(out, \"%s.(none)\", buf);\n>> +\telse {\n>> +\t\t if (canonical_name(buf,out) != 0) strbuf_addf(out, \"%s.(none)\", buf);\n>> +\t}\n>\n> We always put conditional bodies on the next line, even if they are\n> one-liners.\n>\n> Here's what I've queued, addressing those. No need to re-send if you're\n> happy with it:\nOk for me. Thank you very much\n>\n> -- >8 --\n> From: Elia Pinto \n> Subject: [PATCH] ident.c: add support for IPv6\n>\n> Add IPv6 support by implementing name resolution with the\n> protocol agnostic getaddrinfo(3) API. The old gethostbyname(3)\n> code is still available when git is compiled with NO_IPV6.\n>\n> Signed-off-by: Elia Pinto \n> Helped-by: Jeff King \n> Helped-by: Eric Sunshine \n> Signed-off-by: Jeff King \n> ---\n> ident.c | 31 +++++++++++++++++++++++++++----\n> 1 file changed, 27 insertions(+), 4 deletions(-)\n>\n> diff --git a/ident.c b/ident.c\n> index 5ff1aad..4e7f99d 100644\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -70,10 +70,35 @@ static int add_mailname_host(struct strbuf *buf)\n> \treturn 0;\n> }\n>\n> +static int canonical_name(const char *host, struct strbuf *out)\n> +{\n> +\tint status = -1;\n> +\n> +#ifndef NO_IPV6\n> +\tstruct addrinfo hints, *ai;\n> +\tmemset (&hints, '\\0', sizeof (hints));\n> +\thints.ai_flags = AI_CANONNAME;\n> +\tif (!getaddrinfo(host, NULL, &hints, &ai)) {\n> +\t\tif (ai && strchr(ai->ai_canonname, '.')) {\n> +\t\t\tstrbuf_addstr(out, ai->ai_canonname);\n> +\t\t\tstatus = 0;\n> +\t\t}\n> +\t\tfreeaddrinfo(ai);\n> +\t}\n> +#else\n> +\tstruct hostent *he = gethostbyname(buf);\n> +\tif (he && strchr(he->h_name, '.')) {\n> +\t\tstrbuf_addstr(out, he->h_name);\n> +\t\tstatus = 0;\n> +\t}\n> +#endif /* NO_IPV6 */\n> +\n> +\treturn status;\n> +}\n> +\n> static void add_domainname(struct strbuf *out)\n> {\n> \tchar buf[1024];\n> -\tstruct hostent *he;\n>\n> \tif (gethostname(buf, sizeof(buf))) {\n> \t\twarning(\"cannot get host name: %s\", strerror(errno));\n> @@ -82,9 +107,7 @@ static void add_domainname(struct strbuf *out)\n> \t}\n> \tif (strchr(buf, '.'))\n> \t\tstrbuf_addstr(out, buf);\n> -\telse if ((he = gethostbyname(buf)) && strchr(he->h_name, '.'))\n> -\t\tstrbuf_addstr(out, he->h_name);\n> -\telse\n> +\telse if (canonical_name(buf, out) < 0)\n> \t\tstrbuf_addf(out, \"%s.(none)\", buf);\n> }\n>\n> --\n> 2.6.3.636.g1460207\n>\n>\n"}]}