[v1] tap: fix net_init_tap() return code

[PATCH] tap: fix net_init_tap() return code

Posted by Steve Sistare 4 months ago

net_init_tap intends to return 0 for success and -1 on error.  However,
when net_init_tap() succeeds for a multi-queue device, it returns 1,
because of this code where ret becomes 1 when g_unix_set_fd_nonblocking
succeeds:

        ret = g_unix_set_fd_nonblocking(fd, true, NULL);
        if (!ret) {
            ... error ...
    free_fail:
        ...
        return ret;

Luckily, the only current call site checks for negative, rather than non-zero:

  net_client_init1()
      if (net_client_init_fun[](...) < 0)

Also, in the unlikely case that g_unix_set_fd_nonblocking fails and returns
false, ret=0 is returned, and net_client_init1 will use a broken interface.

Fix it to be future proof.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 net/tap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index ae1c7e3..35552c4 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -854,8 +854,8 @@ int net_init_tap(const Netdev *netdev, const char *name,
                 goto free_fail;
             }
 
-            ret = g_unix_set_fd_nonblocking(fd, true, NULL);
-            if (!ret) {
+            if (!g_unix_set_fd_nonblocking(fd, true, NULL)) {
+                ret = -1;
                 error_setg_errno(errp, errno, "%s: Can't use file descriptor %d",
                                  name, fd);
                 goto free_fail;
-- 
1.8.3.1

Re: [PATCH] tap: fix net_init_tap() return code

Posted by Philippe Mathieu-Daudé 4 months ago

On 14/7/25 22:36, Steve Sistare wrote:
> net_init_tap intends to return 0 for success and -1 on error.  However,
> when net_init_tap() succeeds for a multi-queue device, it returns 1,
> because of this code where ret becomes 1 when g_unix_set_fd_nonblocking
> succeeds:
> 
>          ret = g_unix_set_fd_nonblocking(fd, true, NULL);
>          if (!ret) {
>              ... error ...
>      free_fail:
>          ...
>          return ret;
> 
> Luckily, the only current call site checks for negative, rather than non-zero:
> 
>    net_client_init1()
>        if (net_client_init_fun[](...) < 0)
> 
> Also, in the unlikely case that g_unix_set_fd_nonblocking fails and returns
> false, ret=0 is returned, and net_client_init1 will use a broken interface.
> 
> Fix it to be future proof.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   net/tap.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>