diff mbox

[2/3] fwts_hwinfo.c: fix strict-aliasing error with old gcc

Message ID 1478005669-3752-2-git-send-email-ernunes@redhat.com
State Accepted
Headers show

Commit Message

Erico Nunes Nov. 1, 2016, 1:07 p.m. UTC
When building the current fwts release with an older gcc 4.4, we get the
following error due to breaking strict-aliasing rules, despite the
comment in the code which states there has been an attempt to solve this
in the past:

  fwts_hwinfo.c: In function ‘fwts_hwinfo_get’:
  fwts_hwinfo.c:420: error: dereferencing pointer ‘sockaddr’ does break
  strict-aliasing rules
  fwts_hwinfo.c:419: note: initialized from here

This new attempt to fix it just makes a copy of the whole "buf.ifr_addr"
to into sockaddr to avoid running into any strict-aliasing problems.
As there are still supported systems with older gcc versions, such as
RHEL6, it is interesting to have this fixed.

This has been built and tested with gcc 6.2.1, gcc 4.8.5 and gcc 4.4.7.

Signed-off-by: Erico Nunes <ernunes@redhat.com>
---
 src/lib/src/fwts_hwinfo.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Colin Ian King Nov. 1, 2016, 1:14 p.m. UTC | #1
On 01/11/16 07:07, Erico Nunes wrote:
> When building the current fwts release with an older gcc 4.4, we get the
> following error due to breaking strict-aliasing rules, despite the
> comment in the code which states there has been an attempt to solve this
> in the past:
> 
>   fwts_hwinfo.c: In function ‘fwts_hwinfo_get’:
>   fwts_hwinfo.c:420: error: dereferencing pointer ‘sockaddr’ does break
>   strict-aliasing rules
>   fwts_hwinfo.c:419: note: initialized from here
> 
> This new attempt to fix it just makes a copy of the whole "buf.ifr_addr"
> to into sockaddr to avoid running into any strict-aliasing problems.
> As there are still supported systems with older gcc versions, such as
> RHEL6, it is interesting to have this fixed.
> 
> This has been built and tested with gcc 6.2.1, gcc 4.8.5 and gcc 4.4.7.
> 
> Signed-off-by: Erico Nunes <ernunes@redhat.com>
> ---
>  src/lib/src/fwts_hwinfo.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c
> index 083b94d..d749cf2 100644
> --- a/src/lib/src/fwts_hwinfo.c
> +++ b/src/lib/src/fwts_hwinfo.c
> @@ -381,7 +381,7 @@ static int fwts_hwinfo_net_get(
>  
>  	while ((d = readdir(dp)) != NULL) {
>  		struct ifreq buf;
> -		struct sockaddr_in *sockaddr;
> +		struct sockaddr_in sockaddr;
>  		fwts_net_config *net_config;
>  
>  		if (d->d_name[0] == '.')
> @@ -416,8 +416,8 @@ static int fwts_hwinfo_net_get(
>  				fwts_log_error(fw, "Cannot get address for device %s.", d->d_name);
>  		}
>  		/* GCC 4.4 is rather overly pedantic in strict aliasing warnings, this avoids it */
> -		sockaddr = (struct sockaddr_in *)&buf.ifr_addr;
> -		net_config->addr = strdup(inet_ntoa((struct in_addr)sockaddr->sin_addr));
> +		memcpy(&sockaddr, &buf.ifr_addr, sizeof(sockaddr));
> +		net_config->addr = strdup(inet_ntoa((struct in_addr)sockaddr.sin_addr));
>  		if (net_config->addr == NULL) {
>  			fwts_log_error(fw, "Cannot allocate net config H/W address.");
>  			fwts_hwinfo_net_free(net_config);
> 
Thanks for sorting this one out.

Acked-by: Colin Ian King <colin.king@canonical.com>
Alex Hung Nov. 1, 2016, 6:21 p.m. UTC | #2
On 2016-11-01 06:07 AM, Erico Nunes wrote:
> When building the current fwts release with an older gcc 4.4, we get the
> following error due to breaking strict-aliasing rules, despite the
> comment in the code which states there has been an attempt to solve this
> in the past:
>
>   fwts_hwinfo.c: In function ‘fwts_hwinfo_get’:
>   fwts_hwinfo.c:420: error: dereferencing pointer ‘sockaddr’ does break
>   strict-aliasing rules
>   fwts_hwinfo.c:419: note: initialized from here
>
> This new attempt to fix it just makes a copy of the whole "buf.ifr_addr"
> to into sockaddr to avoid running into any strict-aliasing problems.
> As there are still supported systems with older gcc versions, such as
> RHEL6, it is interesting to have this fixed.
>
> This has been built and tested with gcc 6.2.1, gcc 4.8.5 and gcc 4.4.7.
>
> Signed-off-by: Erico Nunes <ernunes@redhat.com>
> ---
>  src/lib/src/fwts_hwinfo.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c
> index 083b94d..d749cf2 100644
> --- a/src/lib/src/fwts_hwinfo.c
> +++ b/src/lib/src/fwts_hwinfo.c
> @@ -381,7 +381,7 @@ static int fwts_hwinfo_net_get(
>
>  	while ((d = readdir(dp)) != NULL) {
>  		struct ifreq buf;
> -		struct sockaddr_in *sockaddr;
> +		struct sockaddr_in sockaddr;
>  		fwts_net_config *net_config;
>
>  		if (d->d_name[0] == '.')
> @@ -416,8 +416,8 @@ static int fwts_hwinfo_net_get(
>  				fwts_log_error(fw, "Cannot get address for device %s.", d->d_name);
>  		}
>  		/* GCC 4.4 is rather overly pedantic in strict aliasing warnings, this avoids it */
> -		sockaddr = (struct sockaddr_in *)&buf.ifr_addr;
> -		net_config->addr = strdup(inet_ntoa((struct in_addr)sockaddr->sin_addr));
> +		memcpy(&sockaddr, &buf.ifr_addr, sizeof(sockaddr));
> +		net_config->addr = strdup(inet_ntoa((struct in_addr)sockaddr.sin_addr));
>  		if (net_config->addr == NULL) {
>  			fwts_log_error(fw, "Cannot allocate net config H/W address.");
>  			fwts_hwinfo_net_free(net_config);
>


Acked-by: Alex Hung <alex.hung@canonical.com>
diff mbox

Patch

diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c
index 083b94d..d749cf2 100644
--- a/src/lib/src/fwts_hwinfo.c
+++ b/src/lib/src/fwts_hwinfo.c
@@ -381,7 +381,7 @@  static int fwts_hwinfo_net_get(
 
 	while ((d = readdir(dp)) != NULL) {
 		struct ifreq buf;
-		struct sockaddr_in *sockaddr;
+		struct sockaddr_in sockaddr;
 		fwts_net_config *net_config;
 
 		if (d->d_name[0] == '.')
@@ -416,8 +416,8 @@  static int fwts_hwinfo_net_get(
 				fwts_log_error(fw, "Cannot get address for device %s.", d->d_name);
 		}
 		/* GCC 4.4 is rather overly pedantic in strict aliasing warnings, this avoids it */
-		sockaddr = (struct sockaddr_in *)&buf.ifr_addr;
-		net_config->addr = strdup(inet_ntoa((struct in_addr)sockaddr->sin_addr));
+		memcpy(&sockaddr, &buf.ifr_addr, sizeof(sockaddr));
+		net_config->addr = strdup(inet_ntoa((struct in_addr)sockaddr.sin_addr));
 		if (net_config->addr == NULL) {
 			fwts_log_error(fw, "Cannot allocate net config H/W address.");
 			fwts_hwinfo_net_free(net_config);