Re: sys_socket: sockaddr_storage

[Simon Josefsson]

Bruno Haible <address@hidden> writes:

> Simon Josefsson wrote:
>> +#define _SS_PADSIZE     (_SS_SIZE - (2 * sizeof (__ss_aligntype)))
>
> If the goal is that sizeof (struct sockaddr_storage) == _SS_SIZE, then the
> formula is incorrect. It should be
>   (_SS_SIZE - (max (sizeof (sa_family_t), alignof (__ss_aligntype)) + sizeof 
> (__ss_aligntype)))

Looks fine.  Thanks.  I'm not sure what the original intention was, but
your interpretation seems likely.

>> +#define _SS_SIZE        128
>
> Is this enough for all platforms? Let's see...
>
> - On platforms like FreeBSD 5.2, MacOS X 10.3, NetBSD 3.0, AIX 5.1, OSF/1 5.1,
>   Haiku, mingw, 'struct sockaddr_storage' is defined with size 128, hence
>   this bound will also be valid for earlier releases of these OSes.
>
> - IRIX 6.5 has no 'struct sockaddr_storage' but all 'struct sockaddr_*' 
> variants
>   are less than 64 bytes in size.
>
> - On OpenBSD 3.8 and HP-UX 11.23, 'struct sockaddr_storage' is defined with a
>   size 256, but actually all 'struct sockaddr_*' variants are less than 64
>    bytes in size.
>
> - But on Solaris 10, 'struct sockaddr_storage' is defined with a size 256,
>   and 'struct sockaddr_dl' actually has size 252.
>
> So I would make _SS_SIZE larger, to 256, at least on Solaris.

Thanks for investigating.  It seems safe to make it 256 on all
platforms.  Wasting a few bytes on non-POSIX platforms isn't a serious
problem.

Complete updated patch below.  Ok to commit?

/Simon

diff --git a/ChangeLog b/ChangeLog
index 3f6d793..599695d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2009-05-07  Simon Josefsson  <address@hidden>
+
+       * modules/sys_socket (Makefile.am): Substitute
+       HAVE_STRUCT_SOCKADDR_STORAGE.
+       * m4/sys_socket_h.m4: Check for sockaddr_storage.
+       * lib/sys_socket.in.h (sockaddr_storage): Define when needed.
+       * tests/test-sys_socket.c: Check sockaddr_storage.
+
 2009-05-04  David Bartley  <address@hidden>
             Bruno Haible  <address@hidden>
 
diff --git a/lib/sys_socket.in.h b/lib/sys_socket.in.h
index 6508bf7..0ad2df1 100644
--- a/lib/sys_socket.in.h
+++ b/lib/sys_socket.in.h
@@ -43,6 +43,23 @@
 #ifndef _GL_SYS_SOCKET_H
 #define _GL_SYS_SOCKET_H
 
+#if address@hidden@
+/* Code taken from glibc sysdeps/unix/sysv/linux/bits/socket.h on
+   2009-05-08, licensed under LGPLv2.1+, plus portability fixes. */
+#define __ss_aligntype  unsigned long int
+#define _SS_SIZE       256
+#define _SS_PADSIZE    (_SS_SIZE - (max (sizeof (sa_family_t),         \
+                                         alignof (__ss_aligntype))     \
+                                    + sizeof (__ss_aligntype)))
+
+struct sockaddr_storage
+{
+  sa_family_t ss_family;      /* Address family, etc.  */
+  __ss_aligntype __ss_align;  /* Force desired alignment.  */
+  char __ss_padding[_SS_PADSIZE];
+};
+#endif
+
 #if @HAVE_SYS_SOCKET_H@
 
 /* A platform that has <sys/socket.h>.  */
diff --git a/m4/sys_socket_h.m4 b/m4/sys_socket_h.m4
index 85a0ace..5bbaad8 100644
--- a/m4/sys_socket_h.m4
+++ b/m4/sys_socket_h.m4
@@ -1,5 +1,5 @@
 # sys_socket_h.m4 serial 12
-dnl Copyright (C) 2005-2008 Free Software Foundation, Inc.
+dnl Copyright (C) 2005-2009 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
 dnl with or without modifications, as long as this notice is preserved.
@@ -40,6 +40,22 @@ AC_DEFUN([gl_HEADER_SYS_SOCKET],
   else
     SYS_SOCKET_H='sys/socket.h'
   fi
+  AC_CHECK_TYPES([struct sockaddr_storage],,,[
+  /* sys/types.h is not needed according to POSIX, but the
+     sys/socket.h in i386-unknown-freebsd4.10 and
+     powerpc-apple-darwin5.5 required it. */
+#include <sys/types.h>
+#ifdef HAVE_SYS_SOCKET_H
+#include <sys/socket.h>
+#endif
+#ifdef HAVE_WS2TCPIP_H
+#include <ws2tcpip.h>
+#endif
+])
+  if test $ac_cv_type_struct_sockaddr_storage = no; then
+    HAVE_STRUCT_SOCKADDR_STORAGE=0
+    SYS_SOCKET_H='sys/socket.h'
+  fi
   if test -n "$SYS_SOCKET_H"; then
     dnl Check prerequisites of the <sys/socket.h> replacement.
     gl_CHECK_NEXT_HEADERS([sys/socket.h])
@@ -114,4 +130,5 @@ AC_DEFUN([gl_SYS_SOCKET_H_DEFAULTS],
   GNULIB_SENDTO=0;      AC_SUBST([GNULIB_SENDTO])
   GNULIB_SETSOCKOPT=0;  AC_SUBST([GNULIB_SETSOCKOPT])
   GNULIB_SHUTDOWN=0;    AC_SUBST([GNULIB_SHUTDOWN])
+  HAVE_STRUCT_SOCKADDR_STORAGE=1; AC_SUBST([HAVE_STRUCT_SOCKADDR_STORAGE])
 ])
diff --git a/modules/sys_socket b/modules/sys_socket
index 011fe69..574bdcc 100644
--- a/modules/sys_socket
+++ b/modules/sys_socket
@@ -44,6 +44,7 @@ sys/socket.h: sys_socket.in.h
              -e 's|@''GNULIB_SHUTDOWN''@|$(GNULIB_SHUTDOWN)|g' \
              -e 's|@''HAVE_WINSOCK2_H''@|$(HAVE_WINSOCK2_H)|g' \
              -e 's|@''HAVE_WS2TCPIP_H''@|$(HAVE_WS2TCPIP_H)|g' \
+             -e 
's|@''HAVE_STRUCT_SOCKADDR_STORAGE''@|$(HAVE_STRUCT_SOCKADDR_STORAGE)|g' \
              -e '/definition of GL_LINK_WARNING/r $(LINK_WARNING_H)' \
              < $(srcdir)/sys_socket.in.h; \
        } > address@hidden
diff --git a/tests/test-sys_socket.c b/tests/test-sys_socket.c
index b7395b3..fe7688e 100644
--- a/tests/test-sys_socket.c
+++ b/tests/test-sys_socket.c
@@ -1,5 +1,5 @@
 /* Test of <sys/socket.h> substitute.
-   Copyright (C) 2007 Free Software Foundation, Inc.
+   Copyright (C) 2007, 2009 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -30,6 +30,8 @@ int a[] = { SHUT_RD, SHUT_WR, SHUT_RDWR };
 int
 main ()
 {
+  struct sockaddr_storage x;
+
   /* Check some errno values.  */
   switch (0)
     {
@@ -43,5 +45,7 @@ main ()
       break;
     }
 
+  x.ss_family = 42;
+
   return 0;
 }