[SSSD] [PATCH] Alignment issues reported by clang.

[Michal Židek]

https://fedorahosted.org/sssd/ticket/1359

This patch mostly silences some alignment related warnings reported by 
clang, but also fixes some real alignment issues.

But not all warnings are suppressed in this patch. These cases still 
generate warnings:

1)=====================================================
src/util/refcount.c:63:25: warning: cast from 'char *' to 'int *' 
increases required alignment from 1 to 4 [-Wcast-align]
     wrapper->refcount = (int *)((char *)wrapper->ptr + refcount_offset);
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/util/refcount.c:82:25: warning: cast from 'char *' to 'int *' 
increases required alignment from 1 to 4 [-Wcast-align]
     wrapper->refcount = (int *)((char *)wrapper->ptr + refcount_offset);
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Comment: This is not a problem, the data we access here is an integer 
that is part of a structure, so there should be no alignment issues (C 
compiler would not create a structure with inaccessible structure 
member). I think suppressing this warning with some additional casts 
would only confuse us later and make the code more obfuscated than it is.

2)=====================================================
src/providers/ldap/sdap_async_sudo_hostinfo.c:233:24: warning: cast from 
'struct sockaddr *' to 'struct sockaddr_in *' increases
       required alignment from 2 to 4 [-Wcast-align]
             ip4_addr = (struct sockaddr_in*)(iface->ifa_addr);
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/providers/ldap/sdap_async_sudo_hostinfo.c:267:27: warning: cast from 
'struct sockaddr *' to 'struct sockaddr_in6 *' increases
       required alignment from 2 to 4 [-Wcast-align]
             ip6_network = (struct sockaddr_in6*)(iface->ifa_netmask);
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...and other sockaddr related casts...

Comment: We only have to make sure that sockaddr is not allocated on 
stack here. But we use the getifaddrs() function for this, so it should 
be OK.

3)=====================================================
src/sss_client/nss_services.c:137:29: warning: cast from 'char *' to 
'char **' increases required alignment from 1 to 8
       [-Wcast-align]
     sr->result->s_aliases = (char **) &(sr->buffer[i+pad]);
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment: Well, there is nothing we can do about this warning. We just 
make sure that the accessed data is aligned to sizeof(char **) using the 
pad variable (in the original code, we aligned it to 32 bits, but that 
was incorrect).

4)=====================================================
src/sss_client/nss_mc_group.c:67:22: warning: cast from 'char *' to 
'char **' increases required alignment from 1 to 8 [-Wcast-align]
     result->gr_mem = (char **)buffer;
                      ^~~~~~~~~~~~~~~

src/monitor/monitor.c:1588:16: warning: cast from 'char *' to 'struct 
inotify_event *' increases required alignment from 1 to 4
       [-Wcast-align]
     in_event = (struct inotify_event *)buf;
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~

src/responder/nss/nsssrv_mmap_cache.c:281:22: warning: cast from 'char 
*' to 'rel_ptr_t *' (aka 'unsigned int *') increases required
       alignment from 1 to 4 [-Wcast-align]
         name_ptr = *((rel_ptr_t *)rec->data);
                      ^~~~~~~~~~~~~~~~~~~~~~

Comment: Here we access the beginnings of allocated memory spaces. So it 
should be OK too. Adding some additional casts just to suppress the 
warnings is here IMO inappropriate (it is then not so clear what data 
type we use).

5)======================================================
src/responder/common/responder_packet.c:79:21: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
     packet->len = &((uint32_t *)packet->buffer)[0];
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:80:21: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
     packet->cmd = &((uint32_t *)packet->buffer)[1];
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:81:24: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
     packet->status = &((uint32_t *)packet->buffer)[2];
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:82:26: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
     packet->reserved = &((uint32_t *)packet->buffer)[3];
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:83:33: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
     packet->body = (uint8_t *)&((uint32_t *)packet->buffer)[4];
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:128:29: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
             packet->len = &((uint32_t *)packet->buffer)[0];
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:129:29: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
             packet->cmd = &((uint32_t *)packet->buffer)[1];
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:130:32: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
             packet->status = &((uint32_t *)packet->buffer)[2];
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:131:34: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
             packet->reserved = &((uint32_t *)packet->buffer)[3];
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/responder/common/responder_packet.c:132:41: warning: cast from 
'uint8_t *' (aka 'unsigned char *') to 'uint32_t *'
       (aka 'unsigned int *') increases required alignment from 1 to 4 
[-Wcast-align]
             packet->body = (uint8_t *)&((uint32_t *)packet->buffer)[4];
                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~

Comment: Same as number 4, but we also use addresses that are multiples 
of the sizeof(data_type_used) (relative to the beginning of the buffer). 
Which is OK.


================================================================
The rest of the warnings are suppressed by this patch.

I also have one question. Does talloc guarantee that the allocated block 
of memory (without the talloc related metadata) is aligned to the 
sizeof(biggest_primitive_C_data_type)? I know malloc does this, but I am 
not sure about talloc. I saw some places in the code, where we rely on 
this feature, so my guess is that talloc does it. Can someone confirm 
it? The above mentioned 3, 4 and 5 also rely on proper aligned memory block.

The patch is in attachment.

Thanks
Michal