To avoid error propagation, let's follow common recommendation to
use return value together with errp.
Probably, it would also be good to use bool as a return type
(switching to true/false as success/failure instead of 0/-1). But
seems almost all functions (including a lot of them with errp
argument) have 0/-1 semantics in net/, so making exclusions doesn't
seem good. If we want such a switch, we should update the whole
net/ directory.
Why do we prefer errp-based functions with return value?
In short with additional return value we get:
- less code to handle error
- don't create and set Error object when it's not required
(when passed errp=NULL)
More details in commit message of e3fe3988d7851cac3
"error: Document Error API usage rules"
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Tested-by: Lei Yang <leiyang@redhat.com>
---
net/tap.c | 55 +++++++++++++++++++++++++------------------------------
1 file changed, 25 insertions(+), 30 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index f37133e301..89feb01756 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -679,11 +679,11 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
#define MAX_TAP_QUEUES 1024
-static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
- const char *model, const char *name,
- const char *ifname, const char *script,
- const char *downscript, const char *vhostfdname,
- int vnet_hdr, int fd, Error **errp)
+static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
+ const char *model, const char *name,
+ const char *ifname, const char *script,
+ const char *downscript, const char *vhostfdname,
+ int vnet_hdr, int fd, Error **errp)
{
Error *err = NULL;
TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
@@ -761,10 +761,11 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
goto failed;
}
- return;
+ return 0;
failed:
qemu_del_net_client(&s->nc);
+ return -1;
}
static int get_fds(char *str, char *fds[], int max)
@@ -801,7 +802,6 @@ int net_init_tap(const Netdev *netdev, const char *name,
/* for the no-fd, no-helper case */
const char *script;
const char *downscript;
- Error *err = NULL;
const char *vhostfdname;
char ifname[128];
int ret = 0;
@@ -846,12 +846,10 @@ int net_init_tap(const Netdev *netdev, const char *name,
return -1;
}
- net_init_tap_one(tap, peer, "tap", name, NULL,
- script, downscript,
- vhostfdname, vnet_hdr, fd, &err);
- if (err) {
- error_propagate(errp, err);
- close(fd);
+ ret = net_init_tap_one(tap, peer, "tap", name, NULL,
+ script, downscript,
+ vhostfdname, vnet_hdr, fd, errp);
+ if (ret < 0) {
return -1;
}
} else if (tap->fds) {
@@ -907,12 +905,11 @@ int net_init_tap(const Netdev *netdev, const char *name,
goto free_fail;
}
- net_init_tap_one(tap, peer, "tap", name, ifname,
- script, downscript,
- tap->vhostfds ? vhost_fds[i] : NULL,
- vnet_hdr, fd, &err);
- if (err) {
- error_propagate(errp, err);
+ ret = net_init_tap_one(tap, peer, "tap", name, ifname,
+ script, downscript,
+ tap->vhostfds ? vhost_fds[i] : NULL,
+ vnet_hdr, fd, errp);
+ if (ret < 0) {
ret = -1;
goto free_fail;
}
@@ -952,11 +949,10 @@ free_fail:
return -1;
}
- net_init_tap_one(tap, peer, "bridge", name, ifname,
- script, downscript, vhostfdname,
- vnet_hdr, fd, &err);
- if (err) {
- error_propagate(errp, err);
+ ret = net_init_tap_one(tap, peer, "bridge", name, ifname,
+ script, downscript, vhostfdname,
+ vnet_hdr, fd, errp);
+ if (ret < 0) {
close(fd);
return -1;
}
@@ -997,12 +993,11 @@ free_fail:
}
}
- net_init_tap_one(tap, peer, "tap", name, ifname,
- i >= 1 ? "no" : script,
- i >= 1 ? "no" : downscript,
- vhostfdname, vnet_hdr, fd, &err);
- if (err) {
- error_propagate(errp, err);
+ ret = net_init_tap_one(tap, peer, "tap", name, ifname,
+ i >= 1 ? "no" : script,
+ i >= 1 ? "no" : downscript,
+ vhostfdname, vnet_hdr, fd, errp);
+ if (ret < 0) {
close(fd);
return -1;
}
--
2.48.1