This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Harald Welte gerrit-no-reply at lists.osmocom.org
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 <pespin at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes