Hi,
-----Original Message----- From: Frank Lichtenheld <fr...@lichtenheld.com> Sent: giovedì 13 aprile 2023 16:19 To: Gert Doering <g...@greenie.muc.de> Cc: Gianmarco De Gregori <gianma...@mandelbit.com>; openvpn-devel@lists.sourceforge.net Subject: Re: [Openvpn-devel] [PATCH v3] Route: add support for user defined routing table On Thu, Apr 13, 2023 at 04:12:32PM +0200, Gert Doering wrote: > Hi, > > On Tue, Apr 04, 2023 at 10:32:26AM +0200, Gianmarco De Gregori wrote: > > diff --git a/src/openvpn/route.c b/src/openvpn/route.c index > > 3798bc65..00419dce 100644 > > --- a/src/openvpn/route.c > > +++ b/src/openvpn/route.c > > @@ -325,7 +325,6 @@ init_route(struct route_ipv4 *r, > > > > CLEAR(*r); > > r->option = ro; > > - > > /* network */ > > > > if (!is_route_parm_defined(ro->network)) > > @@ -437,6 +436,27 @@ init_route(struct route_ipv4 *r, > > > > r->flags |= RT_DEFINED; > > > > + /* routing table id */ > > + > > + r->table_id = 0; > > + if (ro->table_id) > > + { > > + r->table_id = atoi(ro->table_id); > > + if (r->table_id < 0) > > + { > > + msg(M_WARN, PACKAGE_NAME "ROUTE: routing table id for > > + network %s (%s) must be >= 0", > > Frank's comments alerted me to this, and this certainly is not the way > to approach it. Syntax checking of the routing table ID must happen > during option parsing (options.c), not in init_route() - so, this > function should be able to rely on ro->table_id being an *int*, and > properly sanitized - "if set, the content is valid". > > Same for IPv6, of course. To be fair, he implemented it in the same way all the other parameters are implemented. That is why I did not complain about that (e.g. compare ro->metric, which is treated exactly the same way). However, I agree with your general sentiment. I completely agree with switching from uint32_t to *int* as I notice that those muliple checks doesn't make any sense. As Frank said, I chose this approach of following how options->route_default_metric and ro->metric are implemented to be consistent with the overall data flow, anyway, in a follow-up patch I could perform a clean-up of all those parameters to be sanitized and managed through options.c . Regards, Gianmarco De Gregori _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel