We are going to switch from TRACE macro to trace points,
this TRACE complicates things, this patch simplifies it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 nbd/client.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/nbd/client.c b/nbd/client.c
index b97143fa60..5a4825ebe0 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -426,14 +426,13 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
     return QIO_CHANNEL(tioc);
 }
 
-
 int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
                           QCryptoTLSCreds *tlscreds, const char *hostname,
                           QIOChannel **outioc,
                           off_t *size, Error **errp)
 {
     char buf[256];
-    uint64_t magic, s;
+    uint64_t nbd_magic, magic, s;
     int rc;
     bool zeroes = true;
 
@@ -461,15 +460,9 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
         goto fail;
     }
 
-    TRACE("Magic is %c%c%c%c%c%c%c%c",
-          qemu_isprint(buf[0]) ? buf[0] : '.',
-          qemu_isprint(buf[1]) ? buf[1] : '.',
-          qemu_isprint(buf[2]) ? buf[2] : '.',
-          qemu_isprint(buf[3]) ? buf[3] : '.',
-          qemu_isprint(buf[4]) ? buf[4] : '.',
-          qemu_isprint(buf[5]) ? buf[5] : '.',
-          qemu_isprint(buf[6]) ? buf[6] : '.',
-          qemu_isprint(buf[7]) ? buf[7] : '.');
+    memcpy(&nbd_magic, buf, 8);
+    nbd_magic = be64_to_cpu(nbd_magic);
+    TRACE("Magic is 0x%" PRIx64, nbd_magic);
 
     if (memcmp(buf, "NBDMAGIC", 8) != 0) {
         error_setg(errp, "Invalid magic received");
-- 
2.11.1

On 06/21/2017 10:34 AM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to switch from TRACE macro to trace points,
> this TRACE complicates things, this patch simplifies it.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  nbd/client.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/nbd/client.c b/nbd/client.c
> index b97143fa60..5a4825ebe0 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -426,14 +426,13 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
>      return QIO_CHANNEL(tioc);
>  }
>  
> -
>  int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,

Spurious whitespace change?

>                            QCryptoTLSCreds *tlscreds, const char *hostname,
>                            QIOChannel **outioc,
>                            off_t *size, Error **errp)
>  {
>      char buf[256];
> -    uint64_t magic, s;
> +    uint64_t nbd_magic, magic, s;

Why do we need two separate variables? Can't you just reuse 'magic'?

>      int rc;
>      bool zeroes = true;
>  
> @@ -461,15 +460,9 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
>          goto fail;
>      }
>  
> -    TRACE("Magic is %c%c%c%c%c%c%c%c",
> -          qemu_isprint(buf[0]) ? buf[0] : '.',
> -          qemu_isprint(buf[1]) ? buf[1] : '.',
> -          qemu_isprint(buf[2]) ? buf[2] : '.',
> -          qemu_isprint(buf[3]) ? buf[3] : '.',
> -          qemu_isprint(buf[4]) ? buf[4] : '.',
> -          qemu_isprint(buf[5]) ? buf[5] : '.',
> -          qemu_isprint(buf[6]) ? buf[6] : '.',
> -          qemu_isprint(buf[7]) ? buf[7] : '.');
> +    memcpy(&nbd_magic, buf, 8);
> +    nbd_magic = be64_to_cpu(nbd_magic);

Do we really need to copy the memory around twice?  Can't we just use:
 magic = ldq_be_p(buf);
and call it good?

> +    TRACE("Magic is 0x%" PRIx64, nbd_magic);
>  
>      if (memcmp(buf, "NBDMAGIC", 8) != 0) {
>          error_setg(errp, "Invalid magic received");
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 29/06/2017 21:36, Eric Blake wrote:
>> +    memcpy(&nbd_magic, buf, 8);
>> +    nbd_magic = be64_to_cpu(nbd_magic);
> Do we really need to copy the memory around twice?  Can't we just use:
>  magic = ldq_be_p(buf);
> and call it good?

Yes.

Paolo