Hi, On Fri, Jan 13, 2023 at 06:57:53PM -0500, selva.n...@gmail.com wrote: > From: Selva Nair <selva.n...@gmail.com> > > - Instead of 0, 1, 2 use RTA_ERROR, RTA_SUCCESS, RTA_EEXIST > as the return code of route addition functions. > > - Also fix a logging error: status -> (status == RTA_SUCCESS) > > Signed-off-by: Selva Nair <selva.n...@gmail.com>
Thanks for that. The patch looks good, and should go into 2.6.0
(so we have an easier-to-understand codebase to base future patches on),
but I have one request...
> status = add_route_service(r, tt);
> - msg(D_ROUTE, "Route addition via service %s", (status == 1) ?
> "succeeded" : "failed");
> + msg(D_ROUTE, "Route addition via service %s", (status ==
> RTA_SUCCESS) ? "succeeded" : "failed");
We have an informal "soft" line length limit of 80 characters, and "hard"
of ~100, if something just looks bad if adding wrapping. These lines
do lend themselve to a very logical wrap
+ msg(D_ROUTE, "Route addition via service %s",
+ (status == RTA_SUCCESS) ? "succeeded" : "failed");
which will keep them under 80 characters, and, I think, also improve
readability.
With other core team members I would just do a quick ask on IRC and
rewrap on-the-fly, but I won't do that without permission.
Can you re-send a v2?
That said, there is something else...
On Fri, Jan 13, 2023 at 06:57:53PM -0500, selva.n...@gmail.com wrote:
> {
> openvpn_snprintf(out, sizeof(out), "%s %s %s", network, netmask,
> gateway);
> }
> - status = management_android_control(management, "ROUTE", out);
> + status = management_android_control(management, "ROUTE", out) ?
> RTA_SUCCESS : RTA_ERROR;
So android gets a 3-state directly on the function return...
> @@ -1694,7 +1699,8 @@ add_route(struct route_ipv4 *r,
> }
>
> argv_msg(D_ROUTE, &argv);
> - status = openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route add
> command failed");
> + bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route add
> command failed");
> + status = ret ? RTA_SUCCESS : RTA_ERROR;
... while the execve() platforms get an extra return code variable...
I'm fine with both approaches, but if we touch all these places at the same
time, let's decide for one approach :-) - given the line length, using the
"bool ret" approach for Android too sounds like a good choice.
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
