Commit ef2e35fc authored by Eric Blake's avatar Eric Blake
Browse files

nbd/client: Drop pointless buf variable 



There's no need to read into a temporary buffer (oversized
since commit 7d3123e1) followed by a byteswap into a uint64_t
to check for a magic number via memcmp(), when the code
immediately below demonstrates reading into the uint64_t then
byteswapping in place and checking for a magic number via
integer math.  What's more, having a different error message
when the server's first reply byte is 0 is unusual - it's no
different from any other wrong magic number, and we already
detected short reads. That whole strlen() issue has been
present and useless since commit 1d45f8b5 in 2010; perhaps it
was leftover debugging (since the correct magic number happens
to be ASCII)?  Make the error messages more consistent and
detailed while touching things.

Signed-off-by: default avatarEric Blake <eblake@redhat.com>
Reviewed-by: default avatarRichard W.M. Jones <rjones@redhat.com>
Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20181215135324.152629-9-eblake@redhat.com>
parent 3c1fa35d

nbd/client.c

+7 −15
Original line number Diff line number Diff line
@@ -733,7 +733,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, 
                          QIOChannel **outioc, NBDExportInfo *info,

                          Error **errp)

{

    char buf[256];

    uint64_t magic;

    int rc;

    bool zeroes = true;
@@ -754,27 +753,20 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, 
        goto fail;

    }



    if (nbd_read(ioc, buf, 8, errp) < 0) {

        error_prepend(errp, "Failed to read data: ");

        goto fail;

    }



    buf[8] = '\0';

    if (strlen(buf) == 0) {

        error_setg(errp, "Server connection closed unexpectedly");

    if (nbd_read(ioc, &magic, sizeof(magic), errp) < 0) {

        error_prepend(errp, "Failed to read initial magic: ");

        goto fail;

    }



    magic = ldq_be_p(buf);

    magic = be64_to_cpu(magic);

    trace_nbd_receive_negotiate_magic(magic);



    if (memcmp(buf, "NBDMAGIC", 8) != 0) {

        error_setg(errp, "Invalid magic received");

    if (magic != NBD_INIT_MAGIC) {

        error_setg(errp, "Bad initial magic received: 0x%" PRIx64, magic);

        goto fail;

    }



    if (nbd_read(ioc, &magic, sizeof(magic), errp) < 0) {

        error_prepend(errp, "Failed to read magic: ");

        error_prepend(errp, "Failed to read server magic: ");

        goto fail;

    }

    magic = be64_to_cpu(magic);
@@ -913,7 +905,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, 
        }

        info->flags = oldflags;

    } else {

        error_setg(errp, "Bad magic received");

        error_setg(errp, "Bad server magic received: 0x%" PRIx64, magic);

        goto fail;

    }

nbd/nbd-internal.h

+2 −1
Original line number Diff line number Diff line
@@ -46,8 +46,9 @@ 
/* Size of oldstyle negotiation */

#define NBD_OLDSTYLE_NEGOTIATE_SIZE (8 + 8 + 8 + 4 + 124)



#define NBD_INIT_MAGIC              0x4e42444d41474943LL /* ASCII "NBDMAGIC" */

#define NBD_REQUEST_MAGIC           0x25609513

#define NBD_OPTS_MAGIC              0x49484156454F5054LL

#define NBD_OPTS_MAGIC              0x49484156454F5054LL /* ASCII "IHAVEOPT" */

#define NBD_CLIENT_MAGIC            0x0000420281861253LL

#define NBD_REP_MAGIC               0x0003e889045565a9LL