{"thread":{"id":"22965","subject":"Portability patches vs 1.7.0.2 [5/6]","startedAt":"2010-03-09T16:19:06Z","lastAt":"2010-03-09T23:26:24Z","messageCount":3,"participants":["Gary V. Vaughan","Erik Faye-Lund","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"136452","messageId":"20100309161906.GF99172@thor.il.thewrittenword.com","threadId":"22965","inReplyTo":null,"subject":"Portability patches vs 1.7.0.2 [5/6]","fromName":"Gary V. Vaughan","fromEmail":"git@mlists.thewrittenword.com","sentAt":"2010-03-09T16:19:06Z","receivedAt":"2010-03-09T16:19:06Z","isPatch":false,"sender":{"key":"git@mlists.thewrittenword.com","avatar":null},"body":"## sockaddr_t->ss_family is not portable\n\nMany of our supported platforms do not have this declaration, for\nexample solaris2.6 thru 2.8 (I can find a complete list of which of\nour supported platforms have this problem).\n\nNormally, I would solve this by importing the relevant modules from\ngnulib and linking in the portable equivalent (hence the name of the\npatch: gnulib.patch).  But in this case, since git is not built using\nAutomake, I decided that it would be easier to simply revert the part\nof the code that was triggering this problem back to what was used in\ngit-1.6.6. So that is what this patch represents.\n\nActually, I'd like some advice on whether this was a good idea?  Was\nthe change to the code in git-1.7.0 introduced in response to a bug\n(latent or reported) that is fixed by the code I reverted in this\npatch?  If so, what is the right way to fix it (assuming you don't\nwant to introduce a dependency on gnulib and Automake into git)?\n\nCheers,\n-- \nGary V. Vaughan (gary@thewrittenword.com)\n\n\nIndex: daemon.c\n===================================================================\n--- daemon.c.orig\n+++ daemon.c\n@@ -588,24 +588,6 @@ static int execute(struct sockaddr *addr\n \treturn -1;\n }\n \n-static int addrcmp(const struct sockaddr_storage *s1,\n-    const struct sockaddr_storage *s2)\n-{\n-\tif (s1->ss_family != s2->ss_family)\n-\t\treturn s1->ss_family - s2->ss_family;\n-\tif (s1->ss_family == AF_INET)\n-\t\treturn memcmp(&((struct sockaddr_in *)s1)->sin_addr,\n-\t\t    &((struct sockaddr_in *)s2)->sin_addr,\n-\t\t    sizeof(struct in_addr));\n-#ifndef NO_IPV6\n-\tif (s1->ss_family == AF_INET6)\n-\t\treturn memcmp(&((struct sockaddr_in6 *)s1)->sin6_addr,\n-\t\t    &((struct sockaddr_in6 *)s2)->sin6_addr,\n-\t\t    sizeof(struct in6_addr));\n-#endif\n-\treturn 0;\n-}\n-\n static int max_connections = 32;\n \n static unsigned int live_children;\n@@ -625,7 +607,7 @@ static void add_child(pid_t pid, struct \n \tnewborn->pid = pid;\n \tmemcpy(&newborn->address, addr, addrlen);\n \tfor (cradle = &firstborn; *cradle; cradle = &(*cradle)->next)\n-\t\tif (!addrcmp(&(*cradle)->address, &newborn->address))\n+\t\tif (!memcmp(&(*cradle)->address, &newborn->address, sizeof(newborn->address)))\n \t\t\tbreak;\n \tnewborn->next = *cradle;\n \t*cradle = newborn;\n@@ -658,7 +640,7 @@ static void kill_some_child(void)\n \t\treturn;\n \n \tfor (; (next = blanket->next); blanket = next)\n-\t\tif (!addrcmp(&blanket->address, &next->address)) {\n+\t\tif (!memcmp(&blanket->address, &next->address, sizeof(next->address))) {\n \t\t\tkill(blanket->pid, SIGTERM);\n \t\t\tbreak;\n \t\t}\n"},{"id":"136455","messageId":"40aa078e1003090908o74593fafw59bd992eda2b2b12@mail.gmail.com","threadId":"22965","inReplyTo":"20100309161906.GF99172@thor.il.thewrittenword.com","subject":"Re: Portability patches vs 1.7.0.2 [5/6]","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-03-09T17:08:15Z","receivedAt":"2010-03-09T17:08:15Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"In the future, please read Documentation/SubmittingPatches before\nsending patches. Patches should be inlined, for one.\n\nOn Tue, Mar 9, 2010 at 5:19 PM, Gary V. Vaughan\n<git@mlists.thewrittenword.com> wrote:\n> Actually, I'd like some advice on whether this was a good idea?  Was\n> the change to the code in git-1.7.0 introduced in response to a bug\n> (latent or reported) that is fixed by the code I reverted in this\n> patch?\n\nI'm the author of commit 15515b73, who introduced that change (due to\na bug). This is something that git-blame could have told you, and you\ncould have CC'ed me. It's pure luck that I had a look at this patch.\n\nSimply doing memcmp will not work, since sockaddr_storage contains\nboth the IP-address and the port, but the port should not be compared.\n\n>  If so, what is the right way to fix it (assuming you don't\n> want to introduce a dependency on gnulib and Automake into git)?\n>\n\nIf ss_family doesn't exist, does that mean that the platform only supports IPv4?\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"136478","messageId":"20100309232624.GE25265@sigill.intra.peff.net","threadId":"22965","inReplyTo":"20100309161906.GF99172@thor.il.thewrittenword.com","subject":"Re: Portability patches vs 1.7.0.2 [5/6]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-09T23:26:24Z","receivedAt":"2010-03-09T23:26:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 09, 2010 at 04:19:06PM +0000, Gary V. Vaughan wrote:\n\n> ## sockaddr_t->ss_family is not portable\n\nI assume you mean sockaddr_storage here?\n\n> Many of our supported platforms do not have this declaration, for\n> example solaris2.6 thru 2.8 (I can find a complete list of which of\n> our supported platforms have this problem).\n\nDid you mean that list to be inclusive? My Solaris 2.8 definitely has\nsockaddr_storage, as it is necessary for IPv6 handling. I don't remember\nwhen IPv6 support was added, though...if it was in a minor release, then\npresumably earlier versions of 2.8 did not.\n\nAt any rate, we should only need it for IPv6 support, so I think the\nright solution would be to include all uses inside an \"#ifndef NO_IPV6\",\nand for the NO_IPV6 case assume it's a sockaddr_in.\n\n-Peff\n"}]}