[Openvpn-devel] networking_sitnl: standardize returned error when no ifindex can be retrieved

Message ID 20220712220917.4114-1-a@unstable.cc
State Rejected
Headers show
Series [Openvpn-devel] networking_sitnl: standardize returned error when no ifindex can be retrieved | expand

Commit Message

Antonio Quartulli July 12, 2022, 12:09 p.m. UTC
if_nametoindex() sets errno to the related error code in case of
failure.

For this reason the caller function should rather propagate this value
instead of coming up with its own.

Right now instead we have:
* return -1
* return -ENOENT
* return errno (positive value!!)

Make all code behave the same and always return -errno in case of
failure. Most common error will still be -ENODEV, when no device with
the provided name could be found.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 src/openvpn/networking_sitnl.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Antonio Quartulli July 13, 2022, 2:08 a.m. UTC | #1
Hi,

let's drop this patch as using errno after another syscall has been made 
is not clean.

Will send a patch that fixes the errno/-errno alone.

Cheers,

On 13/07/2022 00:09, Antonio Quartulli wrote:
> if_nametoindex() sets errno to the related error code in case of
> failure.
> 
> For this reason the caller function should rather propagate this value
> instead of coming up with its own.
> 
> Right now instead we have:
> * return -1
> * return -ENOENT
> * return errno (positive value!!)
> 
> Make all code behave the same and always return -errno in case of
> failure. Most common error will still be -ENODEV, when no device with
> the provided name could be found.
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
>   src/openvpn/networking_sitnl.c | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
> index b2f3ac72..29b7d8c2 100644
> --- a/src/openvpn/networking_sitnl.c
> +++ b/src/openvpn/networking_sitnl.c
> @@ -660,7 +660,7 @@ net_iface_up(openvpn_net_ctx_t *ctx, const char *iface, bool up)
>       {
>           msg(M_WARN, "%s: rtnl: cannot get ifindex for %s: %s", __func__, iface,
>               strerror(errno));
> -        return -ENOENT;
> +        return -errno;
>       }
>   
>       req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
> @@ -698,7 +698,7 @@ net_iface_mtu_set(openvpn_net_ctx_t *ctx, const char *iface,
>       {
>           msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
>               iface);
> -        return -1;
> +        return -errno;
>       }
>   
>       req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
> @@ -731,7 +731,7 @@ net_addr_ll_set(openvpn_net_ctx_t *ctx, const openvpn_net_iface_t *iface,
>       {
>           msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
>               iface);
> -        return -1;
> +        return -errno;
>       }
>   
>       req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
> @@ -839,7 +839,7 @@ sitnl_addr_ptp_add(sa_family_t af_family, const char *iface,
>       {
>           msg(M_WARN, "%s: cannot get ifindex for %s: %s", __func__, np(iface),
>               strerror(errno));
> -        return -ENOENT;
> +        return -errno;
>       }
>   
>       return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, ifindex,
> @@ -872,7 +872,7 @@ sitnl_addr_ptp_del(sa_family_t af_family, const char *iface,
>       if (ifindex == 0)
>       {
>           msg(M_WARN | M_ERRNO, "%s: cannot get ifindex for %s", __func__, iface);
> -        return -ENOENT;
> +        return -errno;
>       }
>   
>       return sitnl_addr_set(RTM_DELADDR, 0, ifindex, af_family, local, NULL, 0);
> @@ -979,7 +979,7 @@ sitnl_addr_add(sa_family_t af_family, const char *iface,
>       {
>           msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
>               iface);
> -        return -ENOENT;
> +        return -errno;
>       }
>   
>       return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, ifindex,
> @@ -1013,7 +1013,7 @@ sitnl_addr_del(sa_family_t af_family, const char *iface, inet_address_t *addr,
>       {
>           msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
>               iface);
> -        return -ENOENT;
> +        return -errno;
>       }
>   
>       return sitnl_addr_set(RTM_DELADDR, 0, ifindex, af_family, addr, NULL,
> @@ -1163,7 +1163,7 @@ sitnl_route_add(const char *iface, sa_family_t af_family, const void *dst,
>           {
>               msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifindex for %s",
>                   __func__, iface);
> -            return -ENOENT;
> +            return -errno;
>           }
>       }
>   
> @@ -1256,7 +1256,7 @@ sitnl_route_del(const char *iface, sa_family_t af_family, inet_address_t *dst,
>           {
>               msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifindex for %s",
>                   __func__, iface);
> -            return -ENOENT;
> +            return -errno;
>           }
>       }
>   
> @@ -1483,7 +1483,7 @@ net_iface_del(openvpn_net_ctx_t *ctx, const char *iface)
>   
>       if (!ifindex)
>       {
> -        return errno;
> +        return -errno;
>       }
>   
>       req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));

Patch

diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index b2f3ac72..29b7d8c2 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -660,7 +660,7 @@  net_iface_up(openvpn_net_ctx_t *ctx, const char *iface, bool up)
     {
         msg(M_WARN, "%s: rtnl: cannot get ifindex for %s: %s", __func__, iface,
             strerror(errno));
-        return -ENOENT;
+        return -errno;
     }
 
     req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
@@ -698,7 +698,7 @@  net_iface_mtu_set(openvpn_net_ctx_t *ctx, const char *iface,
     {
         msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
             iface);
-        return -1;
+        return -errno;
     }
 
     req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
@@ -731,7 +731,7 @@  net_addr_ll_set(openvpn_net_ctx_t *ctx, const openvpn_net_iface_t *iface,
     {
         msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
             iface);
-        return -1;
+        return -errno;
     }
 
     req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
@@ -839,7 +839,7 @@  sitnl_addr_ptp_add(sa_family_t af_family, const char *iface,
     {
         msg(M_WARN, "%s: cannot get ifindex for %s: %s", __func__, np(iface),
             strerror(errno));
-        return -ENOENT;
+        return -errno;
     }
 
     return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, ifindex,
@@ -872,7 +872,7 @@  sitnl_addr_ptp_del(sa_family_t af_family, const char *iface,
     if (ifindex == 0)
     {
         msg(M_WARN | M_ERRNO, "%s: cannot get ifindex for %s", __func__, iface);
-        return -ENOENT;
+        return -errno;
     }
 
     return sitnl_addr_set(RTM_DELADDR, 0, ifindex, af_family, local, NULL, 0);
@@ -979,7 +979,7 @@  sitnl_addr_add(sa_family_t af_family, const char *iface,
     {
         msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
             iface);
-        return -ENOENT;
+        return -errno;
     }
 
     return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, ifindex,
@@ -1013,7 +1013,7 @@  sitnl_addr_del(sa_family_t af_family, const char *iface, inet_address_t *addr,
     {
         msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
             iface);
-        return -ENOENT;
+        return -errno;
     }
 
     return sitnl_addr_set(RTM_DELADDR, 0, ifindex, af_family, addr, NULL,
@@ -1163,7 +1163,7 @@  sitnl_route_add(const char *iface, sa_family_t af_family, const void *dst,
         {
             msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifindex for %s",
                 __func__, iface);
-            return -ENOENT;
+            return -errno;
         }
     }
 
@@ -1256,7 +1256,7 @@  sitnl_route_del(const char *iface, sa_family_t af_family, inet_address_t *dst,
         {
             msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifindex for %s",
                 __func__, iface);
-            return -ENOENT;
+            return -errno;
         }
     }
 
@@ -1483,7 +1483,7 @@  net_iface_del(openvpn_net_ctx_t *ctx, const char *iface)
 
     if (!ifindex)
     {
-        return errno;
+        return -errno;
     }
 
     req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));