util: standardize return from functions calling virNetlinkCommand

There are several functions that call virNetlinkCommand, and they all
follow a common pattern, with three exit labels: err_exit (or
cleanup), malformed_resp, and buffer_too_small. All three of these
labels do their own cleanup and have their own return. However, the
malformed_resp label usually frees the same items as the
cleanup/err_exit label, and the buffer_too_small label just doesn't
free recvbuf (because it's known to always be NULL at the time we goto
buffer_too_small.

In order to simplify and standardize the code, I've made the following
changes to all of these functions:

1) err_exit is replaced with the more libvirt-ish "cleanup", which
   makes sense because in all cases this code is also executed in the
   case of success, so labelling it err_exit may be confusing.

2) rc is initialized to -1, and set to 0 just before the cleanup
   label. Any code that currently sets rc = -1 is made to instead goto
   cleanup.

3) malformed_resp and buffer_too_small just log their error and goto
   cleanup. This gives us a single return path, and a single place to
   free up resources.

4) In one instance, rather then logging an error immediately, a char*
   msg was pointed to an error string, then goto cleanup (and cleanup
   would log an error if msg != NULL). It takes no more lines of code
   to just log the message as we encounter it.

This patch should have 0 functional effects.
This commit is contained in:
Laine Stump 2012-03-07 12:44:56 -05:00
parent f985773d06
commit 0208face59
3 changed files with 60 additions and 102 deletions

View File

@ -1249,7 +1249,7 @@ virNetDevLinkDump(const char *ifname, int ifindex,
unsigned char **recvbuf,
uint32_t (*getPidFunc)(void))
{
int rc = 0;
int rc = -1;
struct nlmsghdr *resp;
struct nlmsgerr *err;
struct ifinfomsg ifinfo = {
@ -1284,15 +1284,12 @@ virNetDevLinkDump(const char *ifname, int ifindex,
if (!nltarget_kernel) {
pid = getPidFunc();
if (pid == 0) {
rc = -1;
goto cleanup;
}
}
if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, pid) < 0) {
rc = -1;
if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, pid) < 0)
goto cleanup;
}
if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL)
goto malformed_resp;
@ -1309,7 +1306,7 @@ virNetDevLinkDump(const char *ifname, int ifindex,
virReportSystemError(-err->error,
_("error dumping %s (%d) interface"),
ifname, ifindex);
rc = -1;
goto cleanup;
}
break;
@ -1324,29 +1321,22 @@ virNetDevLinkDump(const char *ifname, int ifindex,
default:
goto malformed_resp;
}
if (rc != 0)
VIR_FREE(*recvbuf);
rc = 0;
cleanup:
if (rc < 0)
VIR_FREE(*recvbuf);
nlmsg_free(nl_msg);
return rc;
malformed_resp:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed netlink response message"));
VIR_FREE(*recvbuf);
return -1;
goto cleanup;
buffer_too_small:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("allocated netlink buffer is too small"));
return -1;
goto cleanup;
}
static int
@ -1457,28 +1447,20 @@ virNetDevSetVfConfig(const char *ifname, int ifindex, int vf,
}
rc = 0;
cleanup:
nlmsg_free(nl_msg);
VIR_FREE(recvbuf);
return rc;
malformed_resp:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed netlink response message"));
VIR_FREE(recvbuf);
return rc;
goto cleanup;
buffer_too_small:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("allocated netlink buffer is too small"));
return rc;
goto cleanup;
}
static int

View File

@ -100,7 +100,7 @@ virNetDevMacVLanCreate(const char *ifname,
uint32_t macvlan_mode,
int *retry)
{
int rc = 0;
int rc = -1;
struct nlmsghdr *resp;
struct nlmsgerr *err;
struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
@ -155,7 +155,6 @@ virNetDevMacVLanCreate(const char *ifname,
nla_nest_end(nl_msg, linkinfo);
if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, 0) < 0) {
rc = -1;
goto cleanup;
}
@ -177,14 +176,13 @@ virNetDevMacVLanCreate(const char *ifname,
case -EEXIST:
*retry = 1;
rc = -1;
break;
goto cleanup;
default:
virReportSystemError(-err->error,
_("error creating %s type of interface"),
type);
rc = -1;
goto cleanup;
}
break;
@ -195,27 +193,21 @@ virNetDevMacVLanCreate(const char *ifname,
goto malformed_resp;
}
rc = 0;
cleanup:
nlmsg_free(nl_msg);
VIR_FREE(recvbuf);
return rc;
malformed_resp:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed netlink response message"));
VIR_FREE(recvbuf);
return -1;
goto cleanup;
buffer_too_small:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("allocated netlink buffer is too small"));
return -1;
goto cleanup;
}
/**
@ -229,7 +221,7 @@ buffer_too_small:
*/
int virNetDevMacVLanDelete(const char *ifname)
{
int rc = 0;
int rc = -1;
struct nlmsghdr *resp;
struct nlmsgerr *err;
struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
@ -251,7 +243,6 @@ int virNetDevMacVLanDelete(const char *ifname)
goto buffer_too_small;
if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, 0) < 0) {
rc = -1;
goto cleanup;
}
@ -270,7 +261,7 @@ int virNetDevMacVLanDelete(const char *ifname)
virReportSystemError(-err->error,
_("error destroying %s interface"),
ifname);
rc = -1;
goto cleanup;
}
break;
@ -281,27 +272,21 @@ int virNetDevMacVLanDelete(const char *ifname)
goto malformed_resp;
}
rc = 0;
cleanup:
nlmsg_free(nl_msg);
VIR_FREE(recvbuf);
return rc;
malformed_resp:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed netlink response message"));
VIR_FREE(recvbuf);
return -1;
goto cleanup;
buffer_too_small:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("allocated netlink buffer is too small"));
return -1;
goto cleanup;
}

View File

@ -179,19 +179,20 @@ virNetDevVPortProfileGetStatus(struct nlattr **tb, int32_t vf,
uint16_t *status)
{
int rc = -1;
const char *msg = NULL;
struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, };
if (vf == PORT_SELF_VF && nltarget_kernel) {
if (tb[IFLA_PORT_SELF]) {
if (nla_parse_nested(tb_port, IFLA_PORT_MAX, tb[IFLA_PORT_SELF],
ifla_port_policy)) {
msg = _("error parsing IFLA_PORT_SELF part");
goto err_exit;
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("error parsing IFLA_PORT_SELF part"));
goto cleanup;
}
} else {
msg = _("IFLA_PORT_SELF is missing");
goto err_exit;
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("IFLA_PORT_SELF is missing"));
goto cleanup;
}
} else {
if (tb[IFLA_VF_PORTS]) {
@ -202,14 +203,17 @@ virNetDevVPortProfileGetStatus(struct nlattr **tb, int32_t vf,
nla_for_each_nested(tb_vf_ports, tb[IFLA_VF_PORTS], rem) {
if (nla_type(tb_vf_ports) != IFLA_VF_PORT) {
msg = _("error while iterating over IFLA_VF_PORTS part");
goto err_exit;
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("error while iterating over "
"IFLA_VF_PORTS part"));
goto cleanup;
}
if (nla_parse_nested(tb_port, IFLA_PORT_MAX, tb_vf_ports,
ifla_port_policy)) {
msg = _("error parsing IFLA_VF_PORT part");
goto err_exit;
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("error parsing IFLA_VF_PORT part"));
goto cleanup;
}
if (instanceId &&
@ -226,13 +230,15 @@ virNetDevVPortProfileGetStatus(struct nlattr **tb, int32_t vf,
}
if (!found) {
msg = _("Could not find netlink response with "
"expected parameters");
goto err_exit;
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Could not find netlink response with "
"expected parameters"));
goto cleanup;
}
} else {
msg = _("IFLA_VF_PORTS is missing");
goto err_exit;
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("IFLA_VF_PORTS is missing"));
goto cleanup;
}
}
@ -245,15 +251,12 @@ virNetDevVPortProfileGetStatus(struct nlattr **tb, int32_t vf,
*status = PORT_PROFILE_RESPONSE_INPROGRESS;
rc = 0;
} else {
msg = _("no IFLA_PORT_RESPONSE found in netlink message");
goto err_exit;
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("no IFLA_PORT_RESPONSE found in netlink message"));
goto cleanup;
}
}
err_exit:
if (msg)
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", msg);
cleanup:
return rc;
}
@ -389,11 +392,11 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex,
if (!nltarget_kernel) {
pid = virNetDevVPortProfileGetLldpadPid();
if (pid == 0)
goto err_exit;
goto cleanup;
}
if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, pid) < 0)
goto err_exit;
goto cleanup;
if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL)
goto malformed_resp;
@ -410,7 +413,7 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex,
virReportSystemError(-err->error,
_("error during virtual port configuration of ifindex %d"),
ifindex);
goto err_exit;
goto cleanup;
}
break;
@ -422,28 +425,20 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex,
}
rc = 0;
err_exit:
cleanup:
nlmsg_free(nl_msg);
VIR_FREE(recvbuf);
return rc;
malformed_resp:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed netlink response message"));
VIR_FREE(recvbuf);
return rc;
goto cleanup;
buffer_too_small:
nlmsg_free(nl_msg);
virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
_("allocated netlink buffer is too small"));
return rc;
goto cleanup;
}
@ -558,12 +553,12 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex,
rc = virNetDevLinkDump(NULL, ifindex, nltarget_kernel, tb,
&recvbuf, virNetDevVPortProfileGetLldpadPid);
if (rc < 0)
goto err_exit;
goto cleanup;
rc = virNetDevVPortProfileGetStatus(tb, vf, instanceId, nltarget_kernel,
is8021Qbg, &status);
if (rc < 0)
goto err_exit;
goto cleanup;
if (status == PORT_PROFILE_RESPONSE_SUCCESS ||
status == PORT_VDP_RESPONSE_SUCCESS) {
break;
@ -589,7 +584,7 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex,
rc = -2;
}
err_exit:
cleanup:
VIR_FREE(recvbuf);
return rc;
@ -634,7 +629,7 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname,
enum virNetDevVPortProfileLinkOp virtPortOp,
bool setlink_only)
{
int rc = 0;
int rc = -1;
int op = PORT_REQUEST_ASSOCIATE;
struct ifla_port_vsi portVsi = {
.vsi_mgr_id = virtPort->u.virtPort8021Qbg.managerID,
@ -650,10 +645,9 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname,
vf = PORT_SELF_VF;
if (virNetDevVPortProfileGetPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname,
&vlanid) < 0) {
rc = -1;
goto err_exit;
if (virNetDevVPortProfileGetPhysdevAndVlan(ifname, &physdev_ifindex,
physdev_ifname, &vlanid) < 0) {
goto cleanup;
}
if (vlanid < 0)
@ -676,8 +670,7 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname,
default:
virNetDevError(VIR_ERR_INTERNAL_ERROR,
_("operation type %d not supported"), virtPortOp);
rc = -1;
goto err_exit;
goto cleanup;
}
rc = virNetDevVPortProfileOpCommon(physdev_ifname, physdev_ifindex,
@ -691,9 +684,7 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname,
vf,
op,
setlink_only);
err_exit:
cleanup:
return rc;
}