On Thursday 19 September 2013 20:58:23 Jozsef Kadlecsik wrote: > On Thu, 19 Sep 2013, Oliver wrote: > > On Thursday 19 September 2013 11:30:50 Jozsef Kadlecsik wrote: > > > On Thu, 19 Sep 2013, Oliver wrote: > > > > > On Tue, 17 Sep 2013, Oliver wrote: > > > > <snip> > > > > > > > > > > @@ -461,6 +462,9 @@ mtype_expire(struct ip_set *set, struct htype > > > > > > *h, > > > > > > u8 > > > > > > nets_length, size_t dsize)> > > > > > > > > > > > > struct mtype_elem *data; > > > > > > u32 i; > > > > > > int j; > > > > > > > > > > > > +#if IPSET_NET_COUNT > 1 > > > > > > + u8 k; > > > > > > +#endif > > > > > > > > > > Please get rid of all these #if .. [#else ...] #endif constructions, > > > > > except in mtype_test_cidrs. The compiler optimizes away the for loop > > > > > when > > > > > IPSET_NET_COUNT == 1. > > > > > > > > Yep, there's a couple of other places where I don't currently see a > > > > way > > > > around it, but anywhere that it doesn't pose a logical problem to > > > > existing types, I've removed it. > > > > > > Where do you think it's required, except in mtype_test_cidrs? > > > > Well actually, I've come to the realisation that it's actually required > > pretty much everywhere that I placed it because of the simple reason > > that in the existing hash types, data->cidr is not a u8 array, but it is > > in hash:net,net obviously. > > Yes, you are quite right, I was blind. > > > So now either we can convert all the existing hash types so that the > > cidr member of their struct is a single-element array, or we keep the > > preprocessor conditions, your choice. > > I don't really like either of them... What about tweaking the CIDR macro, > into something like this: > > #ifdef IP_SET_HASH_WITH_NETS > #if IPSET_NET_COUNT > 1 > #define __CIDR(data, i) ((data)->cidr[i]) > #else > #define __CIDR(data, i) ((data)->cidr) > #endif > #ifdef IP_SET_HASH_WITH_NETS_PACKED > /* When cidr is packed with nomatch, cidr - 1 is stored in the entry */ > #define CIDR(data, i) (__CIDR(data, i) + 1) > #else > #define CIDR(data, i) __CIDR(data, i) > #endif > #endif > > What do you think? Yeah, that works for me. I simplified __CIDR for multiple nets to just the following: #define __CIDR(cidr, i) (cidr[i]) So the changes to the rest of the code are pretty minimal :) I'll patch-bomb the mailing list shortly. Kind Regards, Oliver. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
