Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/sys



On 03/02/2021 10:03, Kamil Rytarowski wrote:
On 03.02.2021 06:51, Roy Marples wrote:
Module Name:	src
Committed By:	roy
Date:		Wed Feb  3 05:51:40 UTC 2021

Modified Files:
	src/sys/net: if_arp.h if_ether.h if_gre.h
	src/sys/netinet: if_ether.h igmp.h in.h ip.h ip6.h ip_carp.h ip_icmp.h
	    ip_mroute.h ip_var.h tcp.h tcp_debug.h tcp_var.h udp.h udp_var.h

Log Message:
Remove __packed from various network structures

They are already network aligned and adding the __packed attribute
just causes needless compiler warnings about accssing members of packed
objects.


This changes the ABI for userland programs. With __packed, these
structures, whenever contain at least 2-byte data, can be placed in a
different location inside another structure now.

#include <sys/cdefs.h>
#include <stdio.h>
#include <stdint.h>
#include <stdalign.h>

struct aaa {
         uint16_t a;
         uint8_t b;
         uint8_t c;
} __packed;

struct aaa2 {
         uint8_t x;
         struct aaa y;
};

struct bbb {
         uint16_t a;
         uint8_t b;
         uint8_t c;
};

struct bbb2 {
         uint8_t x;
         struct bbb y;
};

Assuming that struct aaa and bbb are from NetBSD headers and aaa2 and bbb2 are your own constructs then you just have yourself to blame.

struct bbb2 {
          uint8_t x;
          struct bbb y;
} __packed;

Makes bbb2 the same as aaa2.

Before I saw your commit, I wanted to ask to revert the following changes:

icmp6: Remove __packed attribute from icmp6 structures

https://github.com/NetBSD/src/commit/427831ba4bdf388aecf3a378de8faf3a4d44a462

ip6: Remove __packed attribute from ip6 structures

https://github.com/NetBSD/src/commit/e82879afd70b0e801e6ee53bd14c27be3dd1738f

The fallout can be subtle and hard to debug. Once, I removed __packed
from one innocent networking structure myself, qemu networking stopped
working at all.

How you use the structure is up to you.
For the record, we were the only BSD to ever apply __packed to these structures and thanks to modern compilers emitting these wonderful warnings it proved to be a bad move.

Roy


Home | Main Index | Thread Index | Old Index