{"thread":{"id":"22991","subject":"[PATCH 5/5] struct sockaddr_storage->ss_family is not portable","startedAt":"2010-03-11T16:37:15Z","lastAt":"2010-04-26T16:55:22Z","messageCount":17,"participants":["Gary V. Vaughan","Martin Storsjö","Jeff King","Brandon Casey"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"136607","messageId":"20100311163715.GE7877@thor.il.thewrittenword.com","threadId":"22991","inReplyTo":null,"subject":"[PATCH 5/5] struct sockaddr_storage->ss_family is not portable","fromName":"Gary V. Vaughan","fromEmail":"git@mlists.thewrittenword.com","sentAt":"2010-03-11T16:37:15Z","receivedAt":"2010-03-11T16:37:15Z","isPatch":true,"sender":{"key":"git@mlists.thewrittenword.com","avatar":null},"body":"Many of our supported platforms do not have this declaration, for\nexample solaris2.6 thru 2.7.  Lack of ss_family implies no IPV6\nsupport, so we can wrap all the ss_family references in an ifndef\nNO_IPV6, and assume sockaddr_in otherwise.\n\nActually, the test for setting NO_IPV6 at configure time is still\ntoo optimistic and I have to manually pass '-DNO_IPV6' in CPPFLAGS\nat build time on aix-5.2.0.0 and earlier, and irix-6.5 and older\nfor them to pick up the right branch.\n---\n daemon.c |    8 +++++++-\n 1 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 6bc1c23..c9ea500 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -591,17 +591,23 @@ static int execute(struct sockaddr *addr)\n static int addrcmp(const struct sockaddr_storage *s1,\n     const struct sockaddr_storage *s2)\n {\n+#ifndef NO_IPV6\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+#else\n+\t/* Assume AF_INET or equivalent for the likes of Solaris 2.7,\n+\t   HP/UX 11.00 and others do not implement ss_family */\n+\treturn memcmp(&((struct sockaddr_in *)s1)->sin_addr,\n+\t    &((struct sockaddr_in *)s2)->sin_addr,\n+\t    sizeof(struct in_addr));\n #endif\n \treturn 0;\n }\n-- \n1.7.0.2\n\n-- \nGary V. Vaughan (gary@thewrittenword.com)\n"},{"id":"136608","messageId":"alpine.DEB.2.00.1003111838260.29993@cone.home.martin.st","threadId":"22991","inReplyTo":"20100311163715.GE7877@thor.il.thewrittenword.com","subject":"Re: [PATCH 5/5] struct sockaddr_storage->ss_family is not portable","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2010-03-11T16:40:37Z","receivedAt":"2010-03-11T16:40:37Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Thu, 11 Mar 2010, Gary V. Vaughan wrote:\n\n> Many of our supported platforms do not have this declaration, for\n> example solaris2.6 thru 2.7.  Lack of ss_family implies no IPV6\n> support, so we can wrap all the ss_family references in an ifndef\n> NO_IPV6, and assume sockaddr_in otherwise.\n\nWhile this probably is ok as such, you can actually do the same without \naccessing the sockaddr_storage->ss_family; just cast it to (const struct \nsockaddr*) and use ->sa_family instead, that should work just as well, as \nfar as I know.\n\n// Martin\n"},{"id":"136626","messageId":"20100311222722.GB18292@sigill.intra.peff.net","threadId":"22991","inReplyTo":"20100311163715.GE7877@thor.il.thewrittenword.com","subject":"Re: [PATCH 5/5] struct sockaddr_storage->ss_family is not portable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-11T22:27:22Z","receivedAt":"2010-03-11T22:27:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 11, 2010 at 04:37:15PM +0000, Gary V. Vaughan wrote:\n\n> Actually, the test for setting NO_IPV6 at configure time is still\n> too optimistic and I have to manually pass '-DNO_IPV6' in CPPFLAGS\n> at build time on aix-5.2.0.0 and earlier, and irix-6.5 and older\n> for them to pick up the right branch.\n\nAre you running 'configure' or just 'make'? If the latter, then the\ndefaults for each platform are defined in the Makefile (e.g., see around\nline 790 of the Makefile where we turn off IPv6 for Solaris 2.7, but not\n2.8). Patches to tweak the defaults for obscure platforms are welcome.\n\nAlso, now that I look at that, we seem to already have a\nNO_SOCKADDR_STORAGE flag that handles this case? Does setting that fix\nyour problem without this patch?\n\n-Peff\n"},{"id":"136630","messageId":"soW-UavLAeDZXqfDG66SVvi7VeuMUeFh0aD4xLpANaibvx4i0auVpw@cipher.nrlssc.navy.mil","threadId":"22991","inReplyTo":"20100311222722.GB18292@sigill.intra.peff.net","subject":"Re: [PATCH 5/5] struct sockaddr_storage->ss_family is not portable","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-03-11T23:57:01Z","receivedAt":"2010-03-11T23:57:01Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 03/11/2010 04:27 PM, Jeff King wrote:\n> On Thu, Mar 11, 2010 at 04:37:15PM +0000, Gary V. Vaughan wrote:\n> \n>> Actually, the test for setting NO_IPV6 at configure time is still\n>> too optimistic and I have to manually pass '-DNO_IPV6' in CPPFLAGS\n>> at build time on aix-5.2.0.0 and earlier, and irix-6.5 and older\n>> for them to pick up the right branch.\n> \n> Are you running 'configure' or just 'make'? If the latter, then the\n> defaults for each platform are defined in the Makefile (e.g., see around\n> line 790 of the Makefile where we turn off IPv6 for Solaris 2.7, but not\n> 2.8). Patches to tweak the defaults for obscure platforms are welcome.\n> \n> Also, now that I look at that, we seem to already have a\n> NO_SOCKADDR_STORAGE flag that handles this case? Does setting that fix\n> your problem without this patch?\n\nNO_SOCKADDR_STORAGE is already set for Solaris 7, and still daemon.c can\nnot be compiled since ss_family started being used, I think in 15515b73.\n\nI should probably speak up when things break, but I tend to remain silent\nif I don't have the time to investigate.\n\n-brandon\n"},{"id":"136637","messageId":"20100312045654.GH7877@thor.il.thewrittenword.com","threadId":"22991","inReplyTo":"alpine.DEB.2.00.1003111838260.29993@cone.home.martin.st","subject":"Re: [PATCH 5/5] struct sockaddr_storage->ss_family is not portable","fromName":"Gary V. Vaughan","fromEmail":"git@mlists.thewrittenword.com","sentAt":"2010-03-12T04:56:54Z","receivedAt":"2010-03-12T04:56:54Z","isPatch":true,"sender":{"key":"git@mlists.thewrittenword.com","avatar":null},"body":"On Thu, Mar 11, 2010 at 06:40:37PM +0200, Martin Storsj? wrote:\n> On Thu, 11 Mar 2010, Gary V. Vaughan wrote:\n> \n> > Many of our supported platforms do not have this declaration, for\n> > example solaris2.6 thru 2.7.  Lack of ss_family implies no IPV6\n> > support, so we can wrap all the ss_family references in an ifndef\n> > NO_IPV6, and assume sockaddr_in otherwise.\n> \n> While this probably is ok as such, you can actually do the same without \n> accessing the sockaddr_storage->ss_family; just cast it to (const struct \n> sockaddr*) and use ->sa_family instead, that should work just as well, as \n> far as I know.\n\nAt least on aix-5.2 it won't be reliable unless you juggle compiler\nswitches just right (I didn't check anywhere else, but the precedent\nfor the bit ordering of the struct members being different is already\nset):\n\n#if defined(COMPAT_43) && !defined(_KERNEL)\nstruct sockaddr {\n        ushort_t        sa_family;      /* address family */\n        char            sa_data[14];    /* up to 14 bytes of direct\naddress */\n};\n\n\n#else\nstruct sockaddr {\n        uchar_t         sa_len;         /* total length */\n        sa_family_t     sa_family;      /* address family */\n        char            sa_data[14];    /* actually longer; address value */\n};\n#endif\n\nCheers,\n   Gary\n-- \nGary V. Vaughan (gary@thewrittenword.com)\n"},{"id":"136651","messageId":"alpine.DEB.2.00.1003120922040.29993@cone.home.martin.st","threadId":"22991","inReplyTo":"20100312045654.GH7877@thor.il.thewrittenword.com","subject":"Re: [PATCH 5/5] struct sockaddr_storage->ss_family is not portable","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2010-03-12T07:24:01Z","receivedAt":"2010-03-12T07:24:01Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Fri, 12 Mar 2010, Gary V. Vaughan wrote:\n\n> On Thu, Mar 11, 2010 at 06:40:37PM +0200, Martin Storsj? wrote:\n> > On Thu, 11 Mar 2010, Gary V. Vaughan wrote:\n> > \n> > > Many of our supported platforms do not have this declaration, for\n> > > example solaris2.6 thru 2.7.  Lack of ss_family implies no IPV6\n> > > support, so we can wrap all the ss_family references in an ifndef\n> > > NO_IPV6, and assume sockaddr_in otherwise.\n> > \n> > While this probably is ok as such, you can actually do the same without \n> > accessing the sockaddr_storage->ss_family; just cast it to (const struct \n> > sockaddr*) and use ->sa_family instead, that should work just as well, as \n> > far as I know.\n> \n> At least on aix-5.2 it won't be reliable unless you juggle compiler\n> switches just right (I didn't check anywhere else, but the precedent\n> for the bit ordering of the struct members being different is already\n> set):\n> \n> #if defined(COMPAT_43) && !defined(_KERNEL)\n> struct sockaddr {\n>         ushort_t        sa_family;      /* address family */\n>         char            sa_data[14];    /* up to 14 bytes of direct\n> address */\n> };\n> \n> \n> #else\n> struct sockaddr {\n>         uchar_t         sa_len;         /* total length */\n>         sa_family_t     sa_family;      /* address family */\n>         char            sa_data[14];    /* actually longer; address value */\n> };\n> #endif\n\nYes, but if the sockaddr struct can be arranged in different ways, the \nother ones (sockaddr_in, sockaddr_storage, sockaddr_in6) must also be \ndefined coherently - you're always supposed to be able to cast an \nsockaddr_in (or any other of them) to a sockaddr and read the sa_family \nfield. As far as I know, at least.\n\n// Martin\n"},{"id":"136871","messageId":"XI3O9HirgFwPkEqC3RdYR4j56mg_uuJQZk1YFST6ukqbKXjgxaqJdNDHwlLXg5R_FVXWmWQSGmg@cipher.nrlssc.navy.mil","threadId":"22991","inReplyTo":"alpine.DEB.2.00.1003120922040.29993@cone.home.martin.st","subject":"[PATCH] daemon.c: avoid accessing ss_family member of struct sockaddr_storage","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T21:03:00Z","receivedAt":"2010-03-15T21:03:00Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nWhen NO_SOCKADDR_STORAGE is set for a platform, either sockaddr_in or\nsockaddr_in6 is used intead.  Neither of which has an ss_family member.\nThey have an sin_family and sin6_family member respectively.  Since the\naddrcmp() function accesses the ss_family member of a sockaddr_storage\nstruct, compilation fails on platforms which define NO_SOCKADDR_STORGAGE.\n\nSince any sockaddr_* structure can be cast to a struct sockaddr and\nhave its sa_family member read, do so here to workaround this issue.\n\nThanks to Martin Storsjö for pointing out the fix, and Gary Vaughan\nfor drawing attention to the issue.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n daemon.c |    8 +++++---\n 1 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 3769b6f..8a52fdc 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -590,9 +590,11 @@ static int execute(struct sockaddr *addr)\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+\tif (((const struct sockaddr*) s1)->sa_family !=\n+\t    ((const struct sockaddr*) s2)->sa_family)\n+\t\treturn ((const struct sockaddr*) s1)->sa_family -\n+\t\t       ((const struct sockaddr*) s2)->sa_family;\n+\tif (((const struct sockaddr*) s1)->sa_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-- \n1.6.6.2\n"},{"id":"136875","messageId":"20100315212915.GB25342@coredump.intra.peff.net","threadId":"22991","inReplyTo":"XI3O9HirgFwPkEqC3RdYR4j56mg_uuJQZk1YFST6ukqbKXjgxaqJdNDHwlLXg5R_FVXWmWQSGmg@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] daemon.c: avoid accessing ss_family member of struct sockaddr_storage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-15T21:29:15Z","receivedAt":"2010-03-15T21:29:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 15, 2010 at 04:03:00PM -0500, Brandon Casey wrote:\n\n> When NO_SOCKADDR_STORAGE is set for a platform, either sockaddr_in or\n> sockaddr_in6 is used intead.  Neither of which has an ss_family member.\n> They have an sin_family and sin6_family member respectively.  Since the\n> addrcmp() function accesses the ss_family member of a sockaddr_storage\n> struct, compilation fails on platforms which define NO_SOCKADDR_STORGAGE.\n> \n> Since any sockaddr_* structure can be cast to a struct sockaddr and\n> have its sa_family member read, do so here to workaround this issue.\n\nDidn't Gary say that AIX 5.2 sticks sa_len at the front of their\nsockaddr?\n\nWe know that whatever we actually have (an actual sockaddr_storage, or a\nsockaddr_in, or a sockaddr_in6) will have the family at the front, so\ncan you just cast it to sa_family_t?\n\nOr am I wrong in assuming that, and on AIX sockaddr_in actually has\nsa_len at the front, so casting to sockaddr does the right thing (and my\nrecommendation above would actually be broken)? The AIX boxen I have\naccess to are all down at the moment.\n\n-Peff\n"},{"id":"136880","messageId":"alpine.DEB.2.00.1003152336520.29993@cone.home.martin.st","threadId":"22991","inReplyTo":"XI3O9HirgFwPkEqC3RdYR4j56mg_uuJQZk1YFST6ukqbKXjgxaqJdNDHwlLXg5R_FVXWmWQSGmg@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] daemon.c: avoid accessing ss_family member of struct sockaddr_storage","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2010-03-15T21:37:53Z","receivedAt":"2010-03-15T21:37:53Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Mon, 15 Mar 2010, Brandon Casey wrote:\n\n> diff --git a/daemon.c b/daemon.c\n> index 3769b6f..8a52fdc 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -590,9 +590,11 @@ static int execute(struct sockaddr *addr)\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> +\tif (((const struct sockaddr*) s1)->sa_family !=\n> +\t    ((const struct sockaddr*) s2)->sa_family)\n> +\t\treturn ((const struct sockaddr*) s1)->sa_family -\n> +\t\t       ((const struct sockaddr*) s2)->sa_family;\n> +\tif (((const struct sockaddr*) s1)->sa_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\nComing to think about it, would it simplify the code even more if the \nfunction were to take a const struct sockaddr* as a parameter instead? \nThat would, on the other hand, require more casts where it's called, \nthough...\n\n// Martin\n"},{"id":"136881","messageId":"alpine.DEB.2.00.1003152338120.29993@cone.home.martin.st","threadId":"22991","inReplyTo":"20100315212915.GB25342@coredump.intra.peff.net","subject":"Re: [PATCH] daemon.c: avoid accessing ss_family member of struct sockaddr_storage","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2010-03-15T21:41:44Z","receivedAt":"2010-03-15T21:41:44Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Mon, 15 Mar 2010, Jeff King wrote:\n\n> On Mon, Mar 15, 2010 at 04:03:00PM -0500, Brandon Casey wrote:\n> \n> > When NO_SOCKADDR_STORAGE is set for a platform, either sockaddr_in or\n> > sockaddr_in6 is used intead.  Neither of which has an ss_family member.\n> > They have an sin_family and sin6_family member respectively.  Since the\n> > addrcmp() function accesses the ss_family member of a sockaddr_storage\n> > struct, compilation fails on platforms which define NO_SOCKADDR_STORGAGE.\n> > \n> > Since any sockaddr_* structure can be cast to a struct sockaddr and\n> > have its sa_family member read, do so here to workaround this issue.\n> \n> Didn't Gary say that AIX 5.2 sticks sa_len at the front of their\n> sockaddr?\n\nYes, but if they have it in sockaddr, they have it in sockaddr_in (and \nshould have it in sockaddr_storage, if it defines such fields at all). \nThose structs should always be defined so that their \nsa_family/ss_family/sin_family/sin6_family fields match.\n\n> We know that whatever we actually have (an actual sockaddr_storage, or a\n> sockaddr_in, or a sockaddr_in6) will have the family at the front, so\n> can you just cast it to sa_family_t?\n> \n> Or am I wrong in assuming that, and on AIX sockaddr_in actually has\n> sa_len at the front, so casting to sockaddr does the right thing (and my\n> recommendation above would actually be broken)? The AIX boxen I have\n> access to are all down at the moment.\n\nGenerally, I don't think one can assume much about the layout of these \nstructs, there may be this sa_len field on some implementations.\n\nBut you should always be able to cast a sockaddr_in or sockaddr_in6 or \nsockaddr_storage to a sockaddr, to examine its sa_family field, in order \nto know what to cast it to.\n\n// Martin\n"},{"id":"136882","messageId":"s0MQZSOEsdBJUhITxC3jwfFJk5PnIEo0WR5z_GEnSOw@cipher.nrlssc.navy.mil","threadId":"22991","inReplyTo":"20100315212915.GB25342@coredump.intra.peff.net","subject":"Re: [PATCH] daemon.c: avoid accessing ss_family member of struct sockaddr_storage","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T21:42:57Z","receivedAt":"2010-03-15T21:42:57Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 03/15/2010 04:29 PM, Jeff King wrote:\n> On Mon, Mar 15, 2010 at 04:03:00PM -0500, Brandon Casey wrote:\n> \n>> When NO_SOCKADDR_STORAGE is set for a platform, either sockaddr_in or\n>> sockaddr_in6 is used intead.  Neither of which has an ss_family member.\n>> They have an sin_family and sin6_family member respectively.  Since the\n>> addrcmp() function accesses the ss_family member of a sockaddr_storage\n>> struct, compilation fails on platforms which define NO_SOCKADDR_STORGAGE.\n>>\n>> Since any sockaddr_* structure can be cast to a struct sockaddr and\n>> have its sa_family member read, do so here to workaround this issue.\n> \n> Didn't Gary say that AIX 5.2 sticks sa_len at the front of their\n> sockaddr?\n> \n> We know that whatever we actually have (an actual sockaddr_storage, or a\n> sockaddr_in, or a sockaddr_in6) will have the family at the front, so\n> can you just cast it to sa_family_t?\n\nI expect that the layout of the sockaddr_* family of structures will\nfollow the layout of struct sockaddr, otherwise they wouldn't be\ncompatible.\n\nIn other words, I think that if struct sockaddr looks like this:\n\n  struct sockaddr {\n        uchar_t         sa_len;         /* total length */\n        sa_family_t     sa_family;      /* address family */\n        char            sa_data[14];    /* actually longer; address value */\n  };\n\nthen somewhere else, struct sockaddr_in looks like this:\n\n  struct sockaddr_in {\n        uchar_t         sin_len;\n        sin_family_t    sin_family;\n        sin_port;\n        sin_addr;\n        ...\n  };\n\n> Or am I wrong in assuming that, and on AIX sockaddr_in actually has\n> sa_len at the front, so casting to sockaddr does the right thing (and my\n> recommendation above would actually be broken)? The AIX boxen I have\n> access to are all down at the moment.\n\nMaybe Gary can check for us... Gary, what does the declaration for\nstruct sockaddr_in look like in your AIX header file?\n\n-brandon\n"},{"id":"136885","messageId":"Ulrh6ePYHqfB90btctT3EMJiuUz4wjLndvupvp0xJR1sBAao-hZxS0PI6-IxWscYhjaEno7FzgY@cipher.nrlssc.navy.mil","threadId":"22991","inReplyTo":"alpine.DEB.2.00.1003152336520.29993@cone.home.martin.st","subject":"[PATCH v2] daemon.c: avoid accessing ss_family member of struct sockaddr_storage","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T22:10:06Z","receivedAt":"2010-03-15T22:10:06Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nWhen NO_SOCKADDR_STORAGE is set for a platform, either sockaddr_in or\nsockaddr_in6 is used intead.  Neither of which has an ss_family member.\nThey have an sin_family and sin6_family member respectively.  Since the\naddrcmp() function accesses the ss_family member of a sockaddr_storage\nstruct, compilation fails on platforms which define NO_SOCKADDR_STORAGE.\n\nSince any sockaddr_* structure can be cast to a struct sockaddr and\nhave its sa_family member read, do so here to workaround this issue.\n\nThanks to Martin Storsjö for pointing out the fix, and Gary Vaughan\nfor drawing attention to the issue.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nOn 03/15/2010 04:37 PM, Martin Storsjö wrote:\n> Coming to think about it, would it simplify the code even more if the \n> function were to take a const struct sockaddr* as a parameter instead? \n> That would, on the other hand, require more casts where it's called, \n> though...\n\nHow about this.\n\n-brandon\n\n\n daemon.c |   11 +++++++----\n 1 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 3769b6f..2e6766f 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -590,14 +590,17 @@ static int execute(struct sockaddr *addr)\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+\tconst struct sockaddr *sa1 = (const struct sockaddr*) s1;\n+\tconst struct sockaddr *sa2 = (const struct sockaddr*) s2;\n+\n+\tif (sa1->sa_family != sa2->sa_family)\n+\t\treturn sa1->sa_family - sa2->sa_family;\n+\tif (sa1->sa_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+\tif (sa1->sa_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-- \n1.6.6.2\n"},{"id":"136957","messageId":"alpine.DEB.2.00.1003160949270.29993@cone.home.martin.st","threadId":"22991","inReplyTo":"Ulrh6ePYHqfB90btctT3EMJiuUz4wjLndvupvp0xJR1sBAao-hZxS0PI6-IxWscYhjaEno7FzgY@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2] daemon.c: avoid accessing ss_family member of struct sockaddr_storage","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2010-03-16T07:52:58Z","receivedAt":"2010-03-16T07:52:58Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"On Mon, 15 Mar 2010, Brandon Casey wrote:\n\n> On 03/15/2010 04:37 PM, Martin Storsjö wrote:\n> > Coming to think about it, would it simplify the code even more if the \n> > function were to take a const struct sockaddr* as a parameter instead? \n> > That would, on the other hand, require more casts where it's called, \n> > though...\n> \n> How about this.\n\nYes, this looks neat!\n\nAcked-by: Martin Storsjö <martin@martin.st>\n\n// Martin"},{"id":"140321","messageId":"20100312151522.GA11943@thor.il.thewrittenword.com","threadId":"22991","inReplyTo":"alpine.DEB.2.00.1003120922040.29993@cone.home.martin.st","subject":"Re: [PATCH 5/5] struct sockaddr_storage->ss_family is not portable","fromName":"Gary V. Vaughan","fromEmail":"git@mlists.thewrittenword.com","sentAt":"2010-04-25T08:36:44Z","receivedAt":"2010-04-25T08:36:44Z","isPatch":true,"sender":{"key":"git@mlists.thewrittenword.com","avatar":null},"body":"Hi Martin,\n\nOn Fri, Mar 12, 2010 at 09:24:01AM +0200, Martin Storsj? wrote:\n> On Fri, 12 Mar 2010, Gary V. Vaughan wrote:\n> > On Thu, Mar 11, 2010 at 06:40:37PM +0200, Martin Storsj? wrote:\n> > > On Thu, 11 Mar 2010, Gary V. Vaughan wrote:\n> > > \n> > > > Many of our supported platforms do not have this declaration, for\n> > > > example solaris2.6 thru 2.7.  Lack of ss_family implies no IPV6\n> > > > support, so we can wrap all the ss_family references in an ifndef\n> > > > NO_IPV6, and assume sockaddr_in otherwise.\n> > > \n> > > While this probably is ok as such, you can actually do the same without \n> > > accessing the sockaddr_storage->ss_family; just cast it to (const struct \n> > > sockaddr*) and use ->sa_family instead, that should work just as well, as \n> > > far as I know.\n> > \n> > At least on aix-5.2 it won't be reliable unless you juggle compiler\n> > switches just right [...]\n> \n> Yes, but if the sockaddr struct can be arranged in different ways, the \n> other ones (sockaddr_in, sockaddr_storage, sockaddr_in6) must also be \n> defined coherently - you're always supposed to be able to cast an \n> sockaddr_in (or any other of them) to a sockaddr and read the sa_family \n> field. As far as I know, at least.\n\nAh, good point.  And now, having tested that on all our machines it\nworks perfectly, and is much more elegant!\n\nI'll resubmit presently.\n\nCheers,\n-- \nGary V. Vaughan (gary@thewrittenword.com)\n"},{"id":"140322","messageId":"20100316065127.GA26370@thor.il.thewrittenword.com","threadId":"22991","inReplyTo":"s0MQZSOEsdBJUhITxC3jwfFJk5PnIEo0WR5z_GEnSOw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] daemon.c: avoid accessing ss_family member of struct sockaddr_storage","fromName":"Gary V. Vaughan","fromEmail":"git@mlists.thewrittenword.com","sentAt":"2010-04-25T08:37:03Z","receivedAt":"2010-04-25T08:37:03Z","isPatch":true,"sender":{"key":"git@mlists.thewrittenword.com","avatar":null},"body":"The git@mlists... address is the one subscribed to this list, to make\nit easy for us to filter list messages into shared folders.  Because\nwe manage so many packages, one or other of us will drop in and out of\ncontact on the relevant lists depending what package we happen to be\nworking on.\n\nAnyway, you can always put a post directly in my INBOX by Cc:ing\ngary@thewrittenword.com and/or gary@gnu.org if you'd like to be sure\nthat I will read something. :)\n\nOn Mon, Mar 15, 2010 at 04:42:57PM -0500, Brandon Casey wrote:\n> I expect that the layout of the sockaddr_* family of structures will\n> follow the layout of struct sockaddr, otherwise they wouldn't be\n> compatible.\n> \n> In other words, I think that if struct sockaddr looks like this:\n> \n>   struct sockaddr {\n>         uchar_t         sa_len;         /* total length */\n>         sa_family_t     sa_family;      /* address family */\n>         char            sa_data[14];    /* actually longer; address value */\n>   };\n> \n> then somewhere else, struct sockaddr_in looks like this:\n> \n>   struct sockaddr_in {\n>         uchar_t         sin_len;\n>         sin_family_t    sin_family;\n>         sin_port;\n>         sin_addr;\n>         ...\n>   };\n\n> On 03/15/2010 04:29 PM, Jeff King wrote:\n> > Or am I wrong in assuming that, and on AIX sockaddr_in actually has\n> > sa_len at the front, so casting to sockaddr does the right thing (and my\n> > recommendation above would actually be broken)? The AIX boxen I have\n> > access to are all down at the moment.\n> \n> Maybe Gary can check for us... Gary, what does the declaration for\n> struct sockaddr_in look like in your AIX header file?\n\n/usr/include/netinet/in.h excerpt:\n\n/*\n * Socket address, internet style.\n */\nstruct sockaddr_in {\n        uchar_t        sin_len;\n        sa_family_t    sin_family;\n        in_port_t      sin_port;\n        struct in_addr sin_addr;\n        uchar_t        sin_zero[8];\n};\n\nCheers,\n-- \nGary V. Vaughan (gary@thewrittenword.com)\n"},{"id":"140357","messageId":"alpine.DEB.2.00.1004252204020.24585@cone.home.martin.st","threadId":"22991","inReplyTo":"20100312151522.GA11943@thor.il.thewrittenword.com","subject":"Re: [PATCH 5/5] struct sockaddr_storage->ss_family is not portable","fromName":"Martin Storsjö","fromEmail":"martin@martin.st","sentAt":"2010-04-25T19:05:16Z","receivedAt":"2010-04-25T19:05:16Z","isPatch":true,"sender":{"key":"martin@martin.st","avatar":"https://avatars.githubusercontent.com/u/69727?v=4"},"body":"Hi Gary,\n\nOn Sun, 25 Apr 2010, Gary V. Vaughan wrote:\n\n> On Fri, Mar 12, 2010 at 09:24:01AM +0200, Martin Storsj? wrote:\n> > \n> > Yes, but if the sockaddr struct can be arranged in different ways, the \n> > other ones (sockaddr_in, sockaddr_storage, sockaddr_in6) must also be \n> > defined coherently - you're always supposed to be able to cast an \n> > sockaddr_in (or any other of them) to a sockaddr and read the sa_family \n> > field. As far as I know, at least.\n> \n> Ah, good point.  And now, having tested that on all our machines it\n> works perfectly, and is much more elegant!\n> \n> I'll resubmit presently.\n\nActually, Brandon Casey already submitted a patch doing this, which is \navailable in master by now, so this issue is all taken care of. :-)\n\n// Martin\n"},{"id":"140426","messageId":"20100426165522.GC28004@thor.il.thewrittenword.com","threadId":"22991","inReplyTo":"alpine.DEB.2.00.1004252204020.24585@cone.home.martin.st","subject":"Re: [PATCH 5/5] struct sockaddr_storage->ss_family is not portable","fromName":"Gary V. Vaughan","fromEmail":"git@mlists.thewrittenword.com","sentAt":"2010-04-26T16:55:22Z","receivedAt":"2010-04-26T16:55:22Z","isPatch":true,"sender":{"key":"git@mlists.thewrittenword.com","avatar":null},"body":"On Sun, Apr 25, 2010 at 10:05:16PM +0300, Martin Storsj? wrote:\n> Hi Gary,\n\nHi Martin,\n\nThere's unfortunately a 1 month time slip between my having written\nthe messages and having figured out why they were being rejected\n(actually 3 things that needed fixing before resends started arriving,\nwhich is why it took so long)...\n\n> On Sun, 25 Apr 2010, Gary V. Vaughan wrote:\n> \n> > On Fri, Mar 12, 2010 at 09:24:01AM +0200, Martin Storsj? wrote:\n> > > \n> > > Yes, but if the sockaddr struct can be arranged in different ways, the \n> > > other ones (sockaddr_in, sockaddr_storage, sockaddr_in6) must also be \n> > > defined coherently - you're always supposed to be able to cast an \n> > > sockaddr_in (or any other of them) to a sockaddr and read the sa_family \n> > > field. As far as I know, at least.\n> > \n> > Ah, good point.  And now, having tested that on all our machines it\n> > works perfectly, and is much more elegant!\n> > \n> > I'll resubmit presently.\n> \n> Actually, Brandon Casey already submitted a patch doing this, which is \n> available in master by now, so this issue is all taken care of. :-)\n\n...and I didn't notice that until I started rebasing the changesets\nagainst a newer release.\n\nExcellent that one issue is now resolved though, I'll repost my\noutstanding patches presently.\n\nCheers,\n-- \nGary V. Vaughan (gary@thewrittenword.com)\n"}]}