Hi guys, so I've looked a little bit at this and can propose something different.
On Wed, Dec 14, 2016 at 02:59:50PM +0000, David CARLIER wrote: > Hi, > > On 14 December 2016 at 14:48, <[email protected]> wrote: > > Hi, thanks for the patch. > > > > Maybe it is more efficient to simply add a "#define _KERNEL", or the > > following code: > > > > #if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) > > #define _KERNEL > > #endif > > > > It is easy (and temptating) but not a good practice ... in my opinion :-) I find this ugly as well and would prefer to avoid it. The main reason it was done is to ensure userspace does *not* use it so we can't claim it to be an accident. > > I'm not sure that src_hlua.c was the good file for adding these > > kinds of defines or includes. Even if these defines are used only in > > this file, I suppose that it should be used in other file (in the > > future). > > > > I was not sure myself, I hesitated to put it in one of the headers ... > let me know what you all think. I'm seeing that in pattern.c we have the same but the entry is called "s6_addr", and nobody has ever complained that pattern.c did not build since 1.5 or so. Thus in my opinion the correct fix would be to replace all those s6_addr32[] with s6_addr[], eg (for 32-bit use) : (*(uint32_t*)&smp->data.u.ipv6.s6_addr[0] Thus it would give this : diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c index 5ac533a..97a4d51 100644 --- a/src/hlua_fcn.c +++ b/src/hlua_fcn.c @@ -1016,14 +1016,16 @@ int hlua_match_addr(lua_State *L) return 1; } } else { - if (((addr1->addr.v6.ip.s6_addr32[0] & addr2->addr.v6.mask.s6_addr32[0]) == - (addr2->addr.v6.ip.s6_addr32[0] & addr1->addr.v6.mask.s6_addr32[0])) && - ((addr1->addr.v6.ip.s6_addr32[1] & addr2->addr.v6.mask.s6_addr32[1]) == - (addr2->addr.v6.ip.s6_addr32[1] & addr1->addr.v6.mask.s6_addr32[1])) && - ((addr1->addr.v6.ip.s6_addr32[2] & addr2->addr.v6.mask.s6_addr32[2]) == - (addr2->addr.v6.ip.s6_addr32[2] & addr1->addr.v6.mask.s6_addr32[2])) && - ((addr1->addr.v6.ip.s6_addr32[3] & addr2->addr.v6.mask.s6_addr32[3]) == - (addr2->addr.v6.ip.s6_addr32[3] & addr1->addr.v6.mask.s6_addr32[3]))) { + int i; + + for (i = 0; i < 16; i += 4) { + if ((*(uint32_t *)&addr1->addr.v6.ip.s6_addr[i] & + *(uint32_t *)&addr2->addr.v6.mask.s6_addr[i]) != + (*(uint32_t *)&addr2->addr.v6.ip.s6_addr[i] & + *(uint32_t *)&addr1->addr.v6.mask.s6_addr[i])) + break; + } + if (i == 16) { lua_pushboolean(L, 1); return 1; } Also I'm a bot bothered by something I don't understand in the current code : we have two masks and we apply one address to the other address' mask. In fact I don't understand the intended operation, even after reading the doc. The doc says : Match two networks. For example "127.0.0.1/32" matchs "127.0.0.0/8". The order of network is not important. And for me this is a perfect example where it's wrong because that doesn't make sense. None of this network "match" the other one. The first one is included in the second one and not the opposite. I suspect we have something deeply wrong there but with a small impact which requires a fix. Can someone enlighten me ? Willy
