[libvirt] [PATCH] macvtap: Fix error return values to -1 instead of 1
Laine Stump
laine at laine.org
Thu Oct 20 18:57:41 UTC 2011
On 10/20/2011 01:46 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu<roprabhu at cisco.com>
>
> Fixes some cases where 1 was being returned instead of -1.
> There are still some inconsistencies in the file with respect
> to what the return variable is initialized to. Can be fixed
> as a separate patch if needed. The scope of this patch is just
> to fix the return value 1. Did some basic sanity test.
This patch hasn't changed the checks of the return code made by callers
to these functions - a spot check showed several that still say "if (rc)
{ ...." rather than "if (rc < 0) { ....". While that is technically
correct, it is inconsistent with the preferred style in libvirt, and I'm
guessing that style is at least partly the reason for making this patch
in the first place :-). As long as you're going to make this change, you
may as well trace back up the call chain for all changed functions and
fix the callers to be consistent.
(I also noticed at least one place that uses "xxx() != 0" instead of
"xxx() < 0". Making all of these comparisons consistent will make it
much easier for someone auditing the code in the future to understand
that the "errors are < 0" convention has been followed for these functions.)
> Signed-off-by: Roopa Prabhu<roprabhu at cisco.com>
> Reported-by: Eric Blake<eblake at redhat.com>
> ---
> src/util/macvtap.c | 22 ++++++++--------------
> 1 files changed, 8 insertions(+), 14 deletions(-)
>
>
> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
> index 7fd6eb5..f8b9d55 100644
> --- a/src/util/macvtap.c
> +++ b/src/util/macvtap.c
> @@ -480,7 +480,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf,
> bool is8021Qbg,
> uint16_t *status)
> {
> - int rc = 1;
> + int rc = -1;
> const char *msg = NULL;
> struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, };
>
> @@ -806,7 +806,7 @@ doPortProfileOpCommon(bool nltarget_kernel,
> _("error %d during port-profile setlink on "
> "interface %s (%d)"),
> status, ifname, ifindex);
> - rc = 1;
> + rc = -1;
> break;
> }
>
> @@ -867,7 +867,7 @@ doPortProfileOp8021Qbg(const char *ifname,
> const virVirtualPortProfileParamsPtr virtPort,
> enum virVirtualPortOp virtPortOp)
> {
> - int rc;
> + int rc = -1;
>
> # ifndef IFLA_VF_PORT_MAX
>
> @@ -877,7 +877,6 @@ doPortProfileOp8021Qbg(const char *ifname,
> (void)virtPortOp;
> macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Kernel VF Port support was missing at compile time."));
> - rc = 1;
>
> # else /* IFLA_VF_PORT_MAX */
>
> @@ -893,10 +892,8 @@ doPortProfileOp8021Qbg(const char *ifname,
> int vf = PORT_SELF_VF;
>
> if (getPhysdevAndVlan(ifname,&physdev_ifindex, physdev_ifname,
> -&vlanid) != 0) {
> - rc = 1;
> +&vlanid) != 0)
> goto err_exit;
> - }
>
> if (vlanid< 0)
> vlanid = 0;
> @@ -918,7 +915,6 @@ doPortProfileOp8021Qbg(const char *ifname,
> default:
> macvtapError(VIR_ERR_INTERNAL_ERROR,
> _("operation type %d not supported"), virtPortOp);
> - rc = 1;
> goto err_exit;
> }
>
> @@ -982,7 +978,7 @@ doPortProfileOp8021Qbh(const char *ifname,
> const unsigned char *vm_uuid,
> enum virVirtualPortOp virtPortOp)
> {
> - int rc;
> + int rc = -1;
>
> # ifndef IFLA_VF_PORT_MAX
>
> @@ -993,7 +989,6 @@ doPortProfileOp8021Qbh(const char *ifname,
> (void)virtPortOp;
> macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Kernel VF Port support was missing at compile time."));
> - rc = 1;
>
> # else /* IFLA_VF_PORT_MAX */
>
> @@ -1008,10 +1003,9 @@ doPortProfileOp8021Qbh(const char *ifname,
> if (rc)
> goto err_exit;
>
> - if (ifaceGetIndex(true, physfndev,&ifindex)< 0) {
> - rc = 1;
> + rc = ifaceGetIndex(true, physfndev,&ifindex);
> + if (rc< 0)
> goto err_exit;
> - }
>
> switch (virtPortOp) {
> case PREASSOCIATE_RR:
> @@ -1059,7 +1053,7 @@ doPortProfileOp8021Qbh(const char *ifname,
> default:
> macvtapError(VIR_ERR_INTERNAL_ERROR,
> _("operation type %d not supported"), virtPortOp);
> - rc = 1;
> + rc = -1;
> }
>
> err_exit:
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
More information about the libvir-list
mailing list