Eric Blake <eblake(a)redhat.com> wrote on 02/03/2011 01:28:36 PM:
On 02/03/2011 11:12 AM, Davidlohr Bueso wrote:
> It wouldn't hurt to add some parenthesis in the following two
similar expression for better readability.
I take it you were using some gcc option about precedence of operations
being confusing between ! and <; including that gcc warning message in
your commit message can aid the discussion.
>
> ---
> src/util/macvtap.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
> index 09d7b78..92147ab 100644
> --- a/src/util/macvtap.c
> +++ b/src/util/macvtap.c
> @@ -1048,8 +1048,8 @@ doPortProfileOpSetLink(bool nltarget_kernel,
>
> memcpy(ifla_vf_mac.mac, macaddr, 6);
>
> - if (!nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac),
> - &ifla_vf_mac) < 0)
> + if ((!nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac),
> + &ifla_vf_mac)) < 0)
The indentation is wrong, too; the &ifla... of the second line should
line up just after nla_put( of the first line. That was botched in
December by commit 013c000 when the lines were reindented.
Actually, you've uncovered a bigger bug, but didn't fix it right.
ohoh, yes, there's a bug. This is how it should look like
---
src/util/macvtap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: libvirt-acl/src/util/macvtap.c
===================================================================
--- libvirt-acl.orig/src/util/macvtap.c
+++ libvirt-acl/src/util/macvtap.c
@@ -1048,8 +1048,8 @@ doPortProfileOpSetLink(bool nltarget_ker
memcpy(ifla_vf_mac.mac, macaddr, 6);
- if (!nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac),
- &ifla_vf_mac) < 0)
+ if (nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac),
+ &ifla_vf_mac) < 0)
goto buffer_too_small;
}
@@ -1060,8 +1060,8 @@ doPortProfileOpSetLink(bool nltarget_ker
.qos = 0,
};
- if (!nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan),
- &ifla_vf_vlan) < 0)
+ if (nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan),
+ &ifla_vf_vlan) < 0)
goto buffer_too_small;
}
Regards,
Stefan