Re: Build failure on PowerPC in pf

  John-Mark,

On Wed, Feb 26, 2014 at 01:20:17PM -0800, John-Mark Gurney wrote:
J> Justin Hibbits wrote this message on Wed, Feb 26, 2014 at 11:12 -0800:
J> > On Wed, Feb 26, 2014 at 10:32 AM, Justin Hibbits <jrh29_at_alumni.cwru.edu> wrote:
J> > > Building on PowerPC I see the following failure:
J> > >
J> > > cc1: warnings being treated as errors
J> > >
J> > > /home/chmeee/freebsd/head/sys/modules/pf/../../netpfil/pf/pf_ioctl.c:
J> > > In function 'pfioctl':
J> > > /home/chmeee/freebsd/head/sys/modules/pf/../../netpfil/pf/pf_ioctl.c:1357:warning:
J> > > cast to pointer from integer of different size [-Wint-to-pointer-cast]
J> > > /home/chmeee/freebsd/head/sys/modules/pf/../../netpfil/pf/pf_ioctl.c:1359:warning:
J> > > cast to pointer from integer of different size [-Wint-to-pointer-cast]
J> > > /home/chmeee/freebsd/head/sys/modules/pf/../../netpfil/pf/pf_ioctl.c:1361:warning:
J> > > cast to pointer from integer of different size [-Wint-to-pointer-cast]
J> > >
J> > > struct pf_rule has counter_u64_t entries, which are actually pointers
J> > > to uint64_t's.  These pointers get assigned from the result of
J> > > counter_u64_fetch(), which returns a uint64_t.  Looks to me like
J> > > there's a bug in here, but I have no idea what to do to fix it.  And
J> > > I'm surprised this hasn't been reported against other 32-bit
J> > > architectures.
J> > 
J> > Replying to myself, it looks like this was broken by r261882.
J> 
J> This comment says it all:
J> 1352 	glebius 	261882 	/*
J> 1353 	  	  	* XXXGL: this is what happens when internal kernel
J> 1354 	  	  	* structures are used as ioctl API structures.
J> 1355 	  	  	*/
J> 
J> So, one way could be to use a union for the states:
J> union {
J> 	struct {
J> 		counter_u64_t states_cur;
J> 		counter_u64_t states_tot;
J> 		counter_u64_t src_nodes;
J> 	} k;
J> 	struct {
J> 		uint64_t states_cur;
J> 		uint64_t states_tot;
J> 		uint64_t src_nodes;
J> 	} u;
J> } u;
J> 
J> The other option is to cast through uintptr_t...
J> 
J> Even though it'd make the code a bit more ugly, I'd vote for the union,
J> since it's designed for what the code is trying to do...

Would above guarantee us that members of "k" won't cross on members
of "u" when we fill them one by one?

	u.states_cur = counter_u64_fetch(k.states_cur);
	u.states_tot = counter_u64_fetch(k.states_tot);

I'd prefer:

	union states_cur {
		counter_u64_t k;
		uint64_t u;
	}
	union states_tot {
		counter_u64_t k;
		uint64_t u;
	}
	union src_nodes {
		counter_u64_t k;
		uint64_t u;
	}

Or am I overcautious?

-- 
Totus tuus, Glebius.