Is this a valid code fix for strict-aliasing rule break?

Question

I'm wanting to build/burn the latest Mellanox Flexboot firmware to my Infiniband ConnectX-3 cards. However, Mellanox have not provided the latest version in binary form. They do however supply the source code.

Unfortunately, when I attempt to compile it I get the following error:

net/udp/dhcp.c: In function 'start_dhcp':
net/udp/dhcp.c:1361:3: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   seed += *( ( uint32_t * )&netdev->ll_addr[j] );
   ^
cc1: all warnings being treated as errors
make: *** [bin/dhcp.o] Error 1
make: *** Waiting for unfinished jobs....
Build failed!

Looking at the code, the following block is the problem. Specifically the seed line. And it's because strict aliasing is on and we're trying to access one type as another.

/**
 * Having both i and j , makes the code more robust
 * in the improbable case that MAX_LL_ADDR_LEN is not a
 * multiple of 4 bytes.
 */
for ( i = 0, j = 0; i < ( MAX_LL_ADDR_LEN >> 2 ); ++i, j += 4 ){
   seed += *( ( uint32_t * )&netdev->ll_addr[j] );
}

My fix which makes it compile.

for ( i = 0, j = 0; i < ( MAX_LL_ADDR_LEN >> 2 ); ++i, j += 4 ){
   uint32_t s;
   memcpy(&s, &netdev->ll_addr[j], 4);
   seed += s;
}

But before I burn it, is this actually equivalent? I think it should be but I'm just wanting a second opinion before I burn something so important.

FYI: ll_addr is defined as:

#define MAX_LL_ADDR_LEN 20

...

/** Link-layer address
 *
 * This is the current link-layer address assigned to the      
 * device.  It can be changed at runtime.
 */
 uint8_t ll_addr[MAX_LL_ADDR_LEN];

Seed is later used here:

 dhcp->xid = random ( seed );

 /* Store DHCP transaction ID for fakedhcp code */
 dhcp_last_xid = dhcp->xid;

So it looks to me this seed thing is just used to seed the random number generator. I'm not really sure if the way they seed it is a terribly good idea, but nontheless it shouldn't harm things and I think my fix should be fine even if it gave out a different answer. So thanks folks for the answers.

Answer 1

In this case, you're safe, so long as ll_addr is an array of char. The increment of j guarantees that the incrementation will only be performed four bytes at a time, which is the width of uint32_t.

In this case, it may be better to use the original implementation and ignore the error, especially if the code was written prior to C99.

Lorehead also makes a good point regarding endianness.