Patch Set 2: (5 comments)
https://gerrit.osmocom.org/#/c/5216/2/ggsn/ggsn.c File ggsn/ggsn.c: Line 350: if (pdp->peer[1]) { /* ipv4v6 */ I think a for-loop iterating over the pdp->peer array would look better than those two copies of identical code PS2, Line 559: sizeof(addr[0])*2 why not simply sizeof(addr) ? it will clear the entire 'addr' and there's no need for complicated syntax Line 568: for (i = 0; i < num_addr; i++) { nothing here ensures that num_addr is actually <=2, right? I would consider the in46a_from_eua() library function independent and you shouldn't make assumptions on the order or the quantity of returned addresses in another part of the code, at leat not blindly in a way that would cause possible security problems and crashes. https://gerrit.osmocom.org/#/c/5216/2/ggsn/icmpv6.c File ggsn/icmpv6.c: Line 196: member = pdp->peer[1]; we blindly assume that peer[1] is IPv6 type? I mean yes, currently in46a_to_eua returns them in that order, but I think we shouldn't make undocumented asumptions about that https://gerrit.osmocom.org/#/c/5216/2/lib/in46_addr.h File lib/in46_addr.h: Line 32: int in46a_to_eua(const struct in46_addr *src, unsigned int size, struct ul66_t *eua); might be nicer to have a separate function for converting multiple addresses and keep the old one? -- To view, visit https://gerrit.osmocom.org/5216 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic820759167fd3bdf329cb11d4b942e903fe50af5 Gerrit-PatchSet: 2 Gerrit-Project: osmo-ggsn Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: Yes
