| Message ID | 20170621153424.16690-6-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
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"); >
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
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");
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(-)