[SSSD] [PATCHES] struct sockaddr casting

Michal Židek mzidek at redhat.com
Tue Oct 1 17:04:19 UTC 2013


On 10/01/2013 06:59 PM, Michal Židek wrote:
> Hello,
>
> these are the patches, that solve the casting of sockaddr structure.
>
> Patch 0001 - Adds function to filter out special ip addresses (loopback,
> multicast, linklocal, broadcast) and also a unit test for it. Used in
> later patches.
>
> Patch 0002 and 0003 - Use the function from patch 0001 and properly copy
> the sockaddr_in structure to avoid alignment warnings.
>
> Patch 0004 - Avoid casting of sockaddr to sockaddr_in in tests.
>
>
> Also here is the discussion from diffrent thread, related to sockaddr
> casting:
>
> // PASTE BEGIN
>
>  >>>>> I tested some compilers and patches work fine with clang and gcc.
>  >>>>> But there are also another compilers in the world
>  >>>>> ant they needn't understand "pragma GCC". This pragma is not
> standardized.
>  >>>>>
>  >>>>>
>  >>>>> src/providers/dp_dyndns.c(145): warning #161: unrecognized #pragma
>  >>>>>    #pragma GCC diagnostic push
>  >>>>>            ^
>  >>>>>
>  >>>>> src/providers/dp_dyndns.c(146): warning #161: unrecognized #pragma
>  >>>>>    #pragma GCC diagnostic ignored "-Wcast-align"
>  >>>>>            ^
>  >>>>>
>  >>>>> src/providers/dp_dyndns.c(206): warning #161: unrecognized #pragma
>  >>>>>    #pragma GCC diagnostic pop
>  >>>>>            ^
>  >>>>>
>  >>>>> If you are 100% sure about alignment, you can define own macro.
>  >>>>>
>  >>>>> Something like this:
>  >>>>> #define ignore_align(var, dest_type) \
>  >>>>>      (dest_type *)((void*)(var))
>  >>>>>
>  >>>>> And usage:
>  >>>>>      wrapper->refcount = ignore_align(wrapper->ptr +
> refcount_offset, int);
>  >>>>>
>  >>>>>
>  >>>>> Does anybody have any better ideas?
>  >>>> Compiler specific #pragma should not be used. And I think in general
>  >>>> #pragma should be used with great care. Currently there is only
> one use
>  >>>> case in sssd in the memcache code to align some structs which make
>  >>>> sense.
>  >>> Indeed, agree 100%
>  >>>
>  >>>> Especially I think ingnoring a warning is not a good idea. If
> currently
>  >>>> no one has a good idea how to handle the sockaddr and in_addr
> structs in
>  >>>> a way which does not produce a warning I would prefer to keep the
>  >>>> warning around until someone has a good idea. Imo a comment in the
> code
>  >>>> that the warning can be ignored would be sufficient for the time
> being.
>  >>> casting sockaddr is a bug, unless you know for sure it was created as
>  >>> sockaddr_un and then simply casted to sockaddr, but in that case I
> would
>  >>> consider the casting to pass it around just a different bug.
>  >>>
>  >>> Please use sockaddr_un anywhere you need to perform casts. Pass around
>  >>> sockaddr_un and cast it only at the time of use when needed.
>  >> It is not possible to use sockaddr_un(or sockaddr_storage).
>  >>
>  >> We use function getifaddrs to retrieve addresses of network interfaces.
>  >>
>  >> Manual page says (man 3 getifaddrs)
>  >>
> --------------------------------------------------------------------------
>  >> int getifaddrs(struct ifaddrs **ifap);
>  >>
>  >>         The  getifaddrs()  function  creates a linked list of
> structures
>  >>         describing the network interfaces of the local system, and
> stores
>  >>         the address of the first item of the list in *ifap.
>  >>         The list consists of ifaddrs structures, defined as follows:
>  >>
>  >>             struct ifaddrs {
>  >>                 struct ifaddrs  *ifa_next;    /* Next item in list */
>  >>                 char            *ifa_name;    /* Name of interface */
>  >>                 unsigned int     ifa_flags;   /* Flags from
> SIOCGIFFLAGS */
>  >>                 struct sockaddr *ifa_addr;    /* Address of
> interface */
>  >>                 ^^^^^^^^^^^^^^^
>  >> We cannot change this type
>  >>                 struct sockaddr *ifa_netmask; /* Netmask of
> interface */
>  >>                 union {
>  >>                     struct sockaddr *ifu_broadaddr;
>  >>                                      /* Broadcast address of
> interface */
>  >>                     struct sockaddr *ifu_dstaddr;
>  >>                                      /* Point-to-point destination
> address */
>  >>                 } ifa_ifu;
>  >>             #define              ifa_broadaddr ifa_ifu.ifu_broadaddr
>  >>             #define              ifa_dstaddr   ifa_ifu.ifu_dstaddr
>  >>                 void            *ifa_data;    /* Address-specific
> data */
>  >>             };
>  >>
>  >>
> --------------------------------------------------------------------------
>  > In this case you should copy the contents of sockaddr to a new variable
>  > of the right type, and not cast.
>  >
>  > Simo.
>  >
> Yes, this is the only way you can deal with these issues.
> I remembered this from my experiments with ELAPI 3 years ago.
> I ran into similar problems with this function and had to copy data.
>

And now with patches.

Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-util-Add-function-to-filter-IP-addresses.patch
Type: text/x-patch
Size: 7440 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131001/e0f3f514/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-dyndns-Use-filter_ipvX_addr-functions.patch
Type: text/x-patch
Size: 3040 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131001/e0f3f514/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-sdap_async_sudo_hostinfo.c-Use-filter_ipvX_addr.patch
Type: text/x-patch
Size: 5097 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131001/e0f3f514/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-tests-Silence-alignment-warning-in-tests.patch
Type: text/x-patch
Size: 1714 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20131001/e0f3f514/attachment-0003.bin>


More information about the sssd-devel mailing list