[Qemu-devel] [PATCH] util/qemu-sockets: revert Yoda Conditions to normal

[Qemu-devel] [PATCH] util/qemu-sockets: revert Yoda Conditions to normal

[Cao jin]

Follow CODING_STYLE

Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 util/qemu-sockets.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Daniel P. Berrange make me realized there is Yoda Conditions in this file,
this file is mixed with both style, since I just touched this file, so,
reverting it is handy to me.

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 5e08723..a07acc5 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -386,7 +386,7 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
         goto err;
     }
 
-    if (0 != (rc = getaddrinfo(addr, port, &ai, &peer))) {
+    if ((rc = getaddrinfo(addr, port, &ai, &peer)) != 0) {
         error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
                    gai_strerror(rc));
         goto err;
@@ -412,7 +412,7 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
         port = "0";
     }
 
-    if (0 != (rc = getaddrinfo(addr, port, &ai, &local))) {
+    if ((rc = getaddrinfo(addr, port, &ai, &local)) != 0) {
         error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
                    gai_strerror(rc));
         goto err;
@@ -472,20 +472,20 @@ InetSocketAddress *inet_parse(const char *str, Error **errp)
     if (str[0] == ':') {
         /* no host given */
         host[0] = '\0';
-        if (1 != sscanf(str, ":%32[^,]%n", port, &pos)) {
+        if (sscanf(str, ":%32[^,]%n", port, &pos) != 1) {
             error_setg(errp, "error parsing port in address '%s'", str);
             goto fail;
         }
     } else if (str[0] == '[') {
         /* IPv6 addr */
-        if (2 != sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos)) {
+        if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos) != 2) {
             error_setg(errp, "error parsing IPv6 address '%s'", str);
             goto fail;
         }
         addr->ipv6 = addr->has_ipv6 = true;
     } else {
         /* hostname or IPv4 addr */
-        if (2 != sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos)) {
+        if (sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos) != 2) {
             error_setg(errp, "error parsing address '%s'", str);
             goto fail;
         }
-- 
2.1.0

Re: [Qemu-devel] [PATCH] util/qemu-sockets: revert Yoda Conditions to normal

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

On 07/28/2016 04:50 AM, Cao jin wrote:
> Follow CODING_STYLE
> 
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> 
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> ---
>  util/qemu-sockets.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> Daniel P. Berrange make me realized there is Yoda Conditions in this file,
> this file is mixed with both style, since I just touched this file, so,
> reverting it is handy to me.
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 5e08723..a07acc5 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -386,7 +386,7 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
>          goto err;
>      }
>  
> -    if (0 != (rc = getaddrinfo(addr, port, &ai, &peer))) {
> +    if ((rc = getaddrinfo(addr, port, &ai, &peer)) != 0) {

CODING_STYLE is currently silent on whether assignment in conditionals
is appropriate, so maybe that should be patched.  But most of our code
prefers:

rc = getaddrinfo();
if (rc != 0) {

rather than assignments in the conditional.  I don't have any strong
opinions on whether that would require a respin or whether your patch is
fine as is.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] util/qemu-sockets: revert Yoda Conditions to normal

[Cao jin]

On 07/28/2016 11:32 PM, Eric Blake wrote:
> On 07/28/2016 04:50 AM, Cao jin wrote:
>> Follow CODING_STYLE
>>
>> Cc: Daniel P. Berrange <berrange@redhat.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>
>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>> ---
>>   util/qemu-sockets.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> Daniel P. Berrange make me realized there is Yoda Conditions in this file,
>> this file is mixed with both style, since I just touched this file, so,
>> reverting it is handy to me.
>>
>> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
>> index 5e08723..a07acc5 100644
>> --- a/util/qemu-sockets.c
>> +++ b/util/qemu-sockets.c
>> @@ -386,7 +386,7 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
>>           goto err;
>>       }
>>
>> -    if (0 != (rc = getaddrinfo(addr, port, &ai, &peer))) {
>> +    if ((rc = getaddrinfo(addr, port, &ai, &peer)) != 0) {
>
> CODING_STYLE is currently silent on whether assignment in conditionals
> is appropriate, so maybe that should be patched.  But most of our code
> prefers:
>
> rc = getaddrinfo();
> if (rc != 0) {
>

I prefer this style too, save seconds to think what's the value of 
assignment expression. Thanks for pointing it out:) v2 is coming.

> rather than assignments in the conditional.  I don't have any strong
> opinions on whether that would require a respin or whether your patch is
> fine as is.
>

-- 
Yours Sincerely,

Cao jin