Re: [Qemu-devel] [PATCH 03/18] nbd: More debug typo fixes, use correct formats

[Alex Bligh]

On 8 Apr 2016, at 23:05, Eric Blake <address@hidden> wrote:

> Clean up some debug message oddities missed earlier; this includes
> both typos, and recognizing that %d is not necessarily compatible
> with uint32_t.
> 
> Signed-off-by: Eric Blake <address@hidden>


Reviewed-by: Alex Bligh <address@hidden>

> ---
> nbd/client.c | 41 ++++++++++++++++++++++-------------------
> nbd/server.c | 44 +++++++++++++++++++++++---------------------
> 2 files changed, 45 insertions(+), 40 deletions(-)
> 
> diff --git a/nbd/client.c b/nbd/client.c
> index 48f2a21..42e4e52 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -109,25 +109,27 @@ static int nbd_handle_reply_err(QIOChannel *ioc, 
> uint32_t opt, uint32_t type,
> 
>     switch (type) {
>     case NBD_REP_ERR_UNSUP:
> -        TRACE("server doesn't understand request %d, attempting fallback",
> -              opt);
> +        TRACE("server doesn't understand request %" PRIx32
> +              ", attempting fallback", opt);
>         result = 0;
>         goto cleanup;
> 
>     case NBD_REP_ERR_POLICY:
> -        error_setg(errp, "Denied by server for option %x", opt);
> +        error_setg(errp, "Denied by server for option %" PRIx32, opt);
>         break;
> 
>     case NBD_REP_ERR_INVALID:
> -        error_setg(errp, "Invalid data length for option %x", opt);
> +        error_setg(errp, "Invalid data length for option %" PRIx32, opt);
>         break;
> 
>     case NBD_REP_ERR_TLS_REQD:
> -        error_setg(errp, "TLS negotiation required before option %x", opt);
> +        error_setg(errp, "TLS negotiation required before option %" PRIx32,
> +                   opt);
>         break;
> 
>     default:
> -        error_setg(errp, "Unknown error code when asking for option %x", 
> opt);
> +        error_setg(errp, "Unknown error code when asking for option %" 
> PRIx32,
> +                   opt);
>         break;
>     }
> 
> @@ -165,7 +167,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, 
> Error **errp)
>     }
>     opt = be32_to_cpu(opt);
>     if (opt != NBD_OPT_LIST) {
> -        error_setg(errp, "Unexpected option type %x expected %x",
> +        error_setg(errp, "Unexpected option type %" PRIx32 " expected %x",
>                    opt, NBD_OPT_LIST);
>         return -1;
>     }
> @@ -207,7 +209,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, 
> Error **errp)
>             return -1;
>         }
>         if (namelen > 255) {
> -            error_setg(errp, "export name length too long %d", namelen);
> +            error_setg(errp, "export name length too long %" PRIu32, 
> namelen);
>             return -1;
>         }
> 
> @@ -234,7 +236,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, 
> Error **errp)
>             g_free(buf);
>         }
>     } else {
> -        error_setg(errp, "Unexpected reply type %x expected %x",
> +        error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
>                    type, NBD_REP_SERVER);
>         return -1;
>     }
> @@ -349,7 +351,7 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
>     }
>     opt = be32_to_cpu(opt);
>     if (opt != NBD_OPT_STARTTLS) {
> -        error_setg(errp, "Unexpected option type %x expected %x",
> +        error_setg(errp, "Unexpected option type %" PRIx32 " expected %x",
>                    opt, NBD_OPT_STARTTLS);
>         return NULL;
>     }
> @@ -361,7 +363,7 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
>     }
>     type = be32_to_cpu(type);
>     if (type != NBD_REP_ACK) {
> -        error_setg(errp, "Server rejected request to start TLS %x",
> +        error_setg(errp, "Server rejected request to start TLS %" PRIx32,
>                    type);
>         return NULL;
>     }
> @@ -373,7 +375,7 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
>     }
>     length = be32_to_cpu(length);
>     if (length != 0) {
> -        error_setg(errp, "Start TLS reponse was not zero %x",
> +        error_setg(errp, "Start TLS response was not zero %" PRIu32,
>                    length);
>         return NULL;
>     }
> @@ -384,7 +386,7 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
>         return NULL;
>     }
>     data.loop = g_main_loop_new(g_main_context_default(), FALSE);
> -    TRACE("Starting TLS hanshake");
> +    TRACE("Starting TLS handshake");
>     qio_channel_tls_handshake(tioc,
>                               nbd_tls_handshake,
>                               &data,
> @@ -474,7 +476,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char 
> *name, uint32_t *flags,
>         }
>         globalflags = be16_to_cpu(globalflags);
>         *flags = globalflags << 16;
> -        TRACE("Global flags are %x", globalflags);
> +        TRACE("Global flags are %" PRIx32, globalflags);
>         if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) {
>             fixedNewStyle = true;
>             TRACE("Server supports fixed new style");
> @@ -550,7 +552,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char 
> *name, uint32_t *flags,
>         }
>         exportflags = be16_to_cpu(exportflags);
>         *flags |= exportflags;
> -        TRACE("Export flags are %x", exportflags);
> +        TRACE("Export flags are %" PRIx16, exportflags);
>     } else if (magic == NBD_CLIENT_MAGIC) {
>         if (name) {
>             error_setg(errp, "Server does not support export names");
> @@ -683,7 +685,8 @@ ssize_t nbd_send_request(QIOChannel *ioc, struct 
> nbd_request *request)
>     ssize_t ret;
> 
>     TRACE("Sending request to server: "
> -          "{ .from = %" PRIu64", .len = %u, .handle = %" PRIu64", .type=%i}",
> +          "{ .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64
> +          ", .type=%" PRIu16 " }",
>           request->from, request->len, request->handle, request->type);
> 
>     cpu_to_be32w((uint32_t*)buf, NBD_REQUEST_MAGIC);
> @@ -732,12 +735,12 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, struct 
> nbd_reply *reply)
> 
>     reply->error = nbd_errno_to_system_errno(reply->error);
> 
> -    TRACE("Got reply: "
> -          "{ magic = 0x%x, .error = %d, handle = %" PRIu64" }",
> +    TRACE("Got reply: { magic = 0x%" PRIx32 ", .error = % " PRId32
> +          ", handle = %" PRIu64" }",
>           magic, reply->error, reply->handle);
> 
>     if (magic != NBD_REPLY_MAGIC) {
> -        LOG("invalid magic (got 0x%x)", magic);
> +        LOG("invalid magic (got 0x%" PRIx32 ")", magic);
>         return -EINVAL;
>     }
>     return 0;
> diff --git a/nbd/server.c b/nbd/server.c
> index e7e4881..81afae2 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -196,7 +196,7 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, 
> uint32_t type, uint32_t opt)
>     uint64_t magic;
>     uint32_t len;
> 
> -    TRACE("Reply opt=%x type=%x", type, opt);
> +    TRACE("Reply opt=%" PRIx32 " type=%" PRIx32, type, opt);
> 
>     magic = cpu_to_be64(NBD_REP_MAGIC);
>     if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
> @@ -226,7 +226,7 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc, 
> NBDExport *exp)
>     uint64_t magic, name_len;
>     uint32_t opt, type, len;
> 
> -    TRACE("Advertizing export name '%s'", exp->name ? exp->name : "");
> +    TRACE("Advertising export name '%s'", exp->name ? exp->name : "");
>     name_len = strlen(exp->name);
>     magic = cpu_to_be64(NBD_REP_MAGIC);
>     if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
> @@ -392,12 +392,12 @@ static int nbd_negotiate_options(NBDClient *client)
>     TRACE("Checking client flags");
>     be32_to_cpus(&flags);
>     if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) {
> -        TRACE("Support supports fixed newstyle handshake");
> +        TRACE("Client supports fixed newstyle handshake");
>         fixedNewstyle = true;
>         flags &= ~NBD_FLAG_C_FIXED_NEWSTYLE;
>     }
>     if (flags != 0) {
> -        TRACE("Unknown client flags 0x%x received", flags);
> +        TRACE("Unknown client flags 0x%" PRIx32 " received", flags);
>         return -EIO;
>     }
> 
> @@ -431,12 +431,12 @@ static int nbd_negotiate_options(NBDClient *client)
>         }
>         length = be32_to_cpu(length);
> 
> -        TRACE("Checking option 0x%x", clientflags);
> +        TRACE("Checking option 0x%" PRIx32, clientflags);
>         if (client->tlscreds &&
>             client->ioc == (QIOChannel *)client->sioc) {
>             QIOChannel *tioc;
>             if (!fixedNewstyle) {
> -                TRACE("Unsupported option 0x%x", clientflags);
> +                TRACE("Unsupported option 0x%" PRIx32, clientflags);
>                 return -EINVAL;
>             }
>             switch (clientflags) {
> @@ -450,7 +450,8 @@ static int nbd_negotiate_options(NBDClient *client)
>                 break;
> 
>             default:
> -                TRACE("Option 0x%x not permitted before TLS", clientflags);
> +                TRACE("Option 0x%" PRIx32 " not permitted before TLS",
> +                      clientflags);
>                 if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
>                     return -EIO;
>                 }
> @@ -488,7 +489,7 @@ static int nbd_negotiate_options(NBDClient *client)
>                 }
>                 break;
>             default:
> -                TRACE("Unsupported option 0x%x", clientflags);
> +                TRACE("Unsupported option 0x%" PRIx32, clientflags);
>                 if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
>                     return -EIO;
>                 }
> @@ -506,7 +507,7 @@ static int nbd_negotiate_options(NBDClient *client)
>                 return nbd_negotiate_handle_export_name(client, length);
> 
>             default:
> -                TRACE("Unsupported option 0x%x", clientflags);
> +                TRACE("Unsupported option 0x%" PRIx32, clientflags);
>                 return -EINVAL;
>             }
>         }
> @@ -647,12 +648,12 @@ static ssize_t nbd_receive_request(QIOChannel *ioc, 
> struct nbd_request *request)
>     request->from  = be64_to_cpup((uint64_t*)(buf + 16));
>     request->len   = be32_to_cpup((uint32_t*)(buf + 24));
> 
> -    TRACE("Got request: "
> -          "{ magic = 0x%x, .type = %d, from = %" PRIu64" , len = %u }",
> +    TRACE("Got request: { magic = 0x%" PRIx32 ", .type = %" PRIx32
> +          ", from = %" PRIu64 " , len = %" PRIu32 " }",
>           magic, request->type, request->from, request->len);
> 
>     if (magic != NBD_REQUEST_MAGIC) {
> -        LOG("invalid magic (got 0x%x)", magic);
> +        LOG("invalid magic (got 0x%" PRIx32 ")", magic);
>         return -EINVAL;
>     }
>     return 0;
> @@ -665,7 +666,8 @@ static ssize_t nbd_send_reply(QIOChannel *ioc, struct 
> nbd_reply *reply)
> 
>     reply->error = system_errno_to_nbd_errno(reply->error);
> 
> -    TRACE("Sending response to client: { .error = %d, handle = %" PRIu64 " 
> }",
> +    TRACE("Sending response to client: { .error = %" PRId32
> +          ", handle = %" PRIu64 " }",
>           reply->error, reply->handle);
> 
>     /* Reply
> @@ -994,7 +996,7 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, 
> struct nbd_request *reque
>     command = request->type & NBD_CMD_MASK_COMMAND;
>     if (command == NBD_CMD_READ || command == NBD_CMD_WRITE) {
>         if (request->len > NBD_MAX_BUFFER_SIZE) {
> -            LOG("len (%u) is larger than max len (%u)",
> +            LOG("len (%" PRIu32" ) is larger than max len (%u)",
>                 request->len, NBD_MAX_BUFFER_SIZE);
>             rc = -EINVAL;
>             goto out;
> @@ -1007,7 +1009,7 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, 
> struct nbd_request *reque
>         }
>     }
>     if (command == NBD_CMD_WRITE) {
> -        TRACE("Reading %u byte(s)", request->len);
> +        TRACE("Reading %" PRIu32 " byte(s)", request->len);
> 
>         if (read_sync(client->ioc, req->data, request->len) != request->len) {
>             LOG("reading from socket failed");
> @@ -1057,10 +1059,10 @@ static void nbd_trip(void *opaque)
>     }
>     command = request.type & NBD_CMD_MASK_COMMAND;
>     if (command != NBD_CMD_DISC && (request.from + request.len) > exp->size) {
> -            LOG("From: %" PRIu64 ", Len: %u, Size: %" PRIu64
> -            ", Offset: %" PRIu64 "\n",
> -                    request.from, request.len,
> -                    (uint64_t)exp->size, (uint64_t)exp->dev_offset);
> +            LOG("From: %" PRIu64 ", Len: %" PRIu32", Size: %" PRIu64
> +                ", Offset: %" PRIu64 "\n",
> +                request.from, request.len,
> +                (uint64_t)exp->size, (uint64_t)exp->dev_offset);
>         LOG("requested operation past EOF--bad client?");
>         goto invalid_request;
>     }
> @@ -1095,7 +1097,7 @@ static void nbd_trip(void *opaque)
>             goto error_reply;
>         }
> 
> -        TRACE("Read %u byte(s)", request.len);
> +        TRACE("Read %" PRIu32" byte(s)", request.len);
>         if (nbd_co_send_reply(req, &reply, request.len) < 0)
>             goto out;
>         break;
> @@ -1162,7 +1164,7 @@ static void nbd_trip(void *opaque)
>         }
>         break;
>     default:
> -        LOG("invalid request type (%u) received", request.type);
> +        LOG("invalid request type (%" PRIu32 ") received", request.type);
>     invalid_request:
>         reply.error = EINVAL;
>     error_reply:
> -- 
> 2.5.5
> 
> 

-- 
Alex Bligh