Page MenuHomeFreeBSD

Use a char * to avoid alignment warnings.
Needs ReviewPublic

Authored by jhb on Mon, Sep 13, 6:23 PM.

Details

Reviewers
emaste
donner
imp
Summary

This fixes a -Waddress-of-packed-member error raised by GCC 9.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 41762
Build 38651: arc lint + arc unit

Event Timeline

jhb requested review of this revision.Mon, Sep 13, 6:23 PM
This revision is now accepted and ready to land.Mon, Sep 13, 7:09 PM
afedorov added inline comments.
tests/sys/netinet/libalias/util.c
111

Just a side note. Most of the networking code uses: (p-> ip_hl << 2)

Perhaps we need an appropriate macro for this.

Which architecture causes this problem?

tests/sys/netinet/libalias/util.c
111–112

I'd prefer uint8_t.

char is not well defined.
It might be 32bit long (seen that).

The build fails with GCC on all architectures.

tests/sys/netinet/libalias/util.c
111–112

FreeBSD doesn't work on architectures where char is not a byte. See all the uses of 'caddr_t'. This is the code in udp_input to calculate the pointer:

	ip = mtod(m, struct ip *);
	uh = (struct udphdr *)((caddr_t)ip + iphlen);

In fact, it'd probably be cleaner to write this instead as

struct udphdr *u = (struct udphdr *)((caddr_t)ip + (ip->ip_hl << 2));

to more closely match udp_input. A hlen helper variable for ip->ip_hl << 2 wouldn't be bad either.

If char is really 32-bits long, then the uint8_t type must necessarily also be at least 32-bits long. char is the smallest unit you can have in C, absent non-standard extensions (and ignoring the special case of bit fields which aren't a first-class type).

In D31941#721219, @imp wrote:

If char is really 32-bits long, then the uint8_t type must necessarily also be at least 32-bits long. char is the smallest unit you can have in C, absent non-standard extensions (and ignoring the special case of bit fields which aren't a first-class type).

A compiler for DSP had this "feature", nevermind. It's just the reason why I prefer "int8_t" over "char".
The proposal from @jhb does please my feeling, so go for it.

  • Update to the pattern used in the kernel.
This revision now requires review to proceed.Sat, Sep 25, 4:56 PM

*sigh* Now this causes a -Wcast-align error with clang even though it's the same exact code as used in udp_input() in the kernel. I guess the kernel doesn't build with -Wcast-align perhaps?