Thanks very much for testing! And pointing out the issue! Both patches have been applied.

> compiling this patch with gcc version 5.3.0 configured for target > arm-poky-linux-gnueabi I get error "comparison between signed and > unsigned integer expressions [-Werror=sign-compare]". In the past I was > using version 4.8.4 and this error did not appear. gcc gets better and better in detecting such things. So it is not surprise to me that a newer version suddenly starts to complain. I tried to figure out how to solve this. It seems the canonical way according the man pages is to use a cast like: - if (expired == 0xffffffff) + if (expired == (time_t)0xffffffff) Wouldn't mind if someone with more knowledge could answer this question.

> In addition but not related with this patch I get: > > /opt/sources/connman/src/iptables.c: In function 'iptables_add_chain': > /opt/sources/connman/src/iptables.c:622:10: error: cast increases required alignment of target type [-Werror=cast-align] > error = (struct error_target *) entry_head->elems; > ^ > /opt/sources/connman/src/iptables.c:640:13: error: cast increases required alignment of target type [-Werror=cast-align] > standard = (struct ipt_standard_target *) entry_return->elems; > ^ > /opt/sources/connman/src/iptables.c: In function 'find_existing_rule': > /opt/sources/connman/src/iptables.c:982:12: error: cast increases required alignment of target type [-Werror=cast-align] > xt_e_m = (struct xt_entry_match *)entry_test->elems; > ^ > /opt/sources/connman/src/iptables.c:1014:17: error: cast increases required alignment of target type [-Werror=cast-align] > tmp_xt_e_m = (struct xt_entry_match *)tmp_e->elems; > ^ > /opt/sources/connman/src/iptables.c: In function 'dump_match': > /opt/sources/connman/src/iptables.c:1275:10: error: cast increases required alignment of target type [-Werror=cast-align] > match = (struct xt_entry_match *) entry->elems; > > Has someone updated the gcc version and getting these errors? We had those buggers in the past once and... commit d482e18ddf59be92427356420def4d8cdc8f2e7d Author: Jukka Rissanen <jukka.rissanen(a)linux.intel.com&gt; Date: Thu Aug 22 16:00:11 2013 +0300 iptables: Alignment fixes The alignment of entry, offset and target was not calculated properly. This by chance worked for x86 but not for ARM. ... fixed a bunch of them. Hmm, I don't know how to fix that, any ideas? A quick peek into the iptables code shows they do it with memcpy (random example): static struct ipt_entry * generate_entry(const struct ipt_entry *fw, struct xtables_rule_match *matches, struct xt_entry_target *target) { unsigned int size; struct xtables_rule_match *matchp; struct ipt_entry *e; size = sizeof(struct ipt_entry); for (matchp = matches; matchp; matchp = matchp->next) size += matchp->match->m->u.match_size; e = xtables_malloc(size + target->u.target_size); *e = *fw; e->target_offset = size; e->next_offset = size + target->u.target_size; size = 0; for (matchp = matches; matchp; matchp = matchp->next) { memcpy(e->elems + size, matchp->match->m, matchp->match->m->u.match_size); size += matchp->match->m->u.match_size; } memcpy(e->elems + size, target, target->u.target_size); return e; } So is memcpy the solution?

>> I tried to figure out how to solve this. It seems the canonical >> way according the man pages is to use a cast like: >> >> - if (expired == 0xffffffff) >> + if (expired == (time_t)0xffffffff) >> >> Wouldn't mind if someone with more knowledge could answer this question. > > > Indeed, that is why I replied to this patch; maybe Patrik or Feng > Wang could tell us if that casting could impact the normal behavior of > this > patch because I also thought the same solution but I do not actually > know the context of this patch. Well, it seem we both are not really sound in the casting theory here. Maybe have a look at the resulting assembly. If our assumption is correct, I would expect the same assembly. > iptables already implements a function to get struct xt_entry_target > pointer from a struct ipt_entry pointer in iptables.h: > > static __inline__ struct xt_entry_target * > ipt_get_target(struct ipt_entry *e) > { > return (void *)e + e->target_offset; > } > > Then, two of the error could be solve doing this: > > - error = (struct error_target *) entry_head->elems; > + error = (struct error_target *) ipt_get_target(entry_head); > > - standard = (struct ipt_standard_target *) entry_return->elems; > + standard = (struct ipt_standard_target *) > ipt_get_target(entry_return); > > Do you agree? That make sense.

> On the other hand, to get the struct xt_entry_match pointer from > ipt_entry we could follow the same approach by defining a function like > this: > > static __inline__ struct xt_entry_match *ipt_get_match(struct ipt_entry > *e) > { > return (void *)e->elems; > } > > What do you think? I would really appreciate if someone with much > more knowledge on this could test those ideas or suggest a possible > solution. > In fact, I am not completely sure if this is actually an alignment > problem or only a casting problem because of different struct type. Yes, let's add this helper function as is defined in iptables.h. Best thing we can do is mimic what iptables does. The 'API' is pretty hard to get right, so it's better to be as close iptables as possible. That code should be okay in respect to alignment. I wont have time to spin a patches today. So I don't mind of you send fixes instead :)