[Qemu-devel] Re: [PATCH] win32: use PRId64 instead of %lld

[Michael S. Tsirkin]

On Sun, Jan 24, 2010 at 09:23:41PM +0000, Herve Poussineau wrote:
> Replace %lld occurrences by PRId64.

This is wrong.
long long values should be printed with %lld.
size_t - with %zd.  PRId64 is for int64_t.

> Incidentally, this fixes use of curl on Windows, and prevents an assert
> when closing Qemu.

It does? How come?

> Signed-off-by: Herve Poussineau <address@hidden>
> ---
>  block/curl.c           |   10 +++++-----
>  block/qcow2.c          |    2 +-
>  hw/vga.c               |    2 +-
>  json-lexer.c           |   16 ++++++++++++++++
>  json-parser.c          |    2 +-
>  qemu-img.c             |    8 ++++----
>  qemu-io.c              |   24 ++++++++++++------------
>  target-ppc/translate.c |    6 +++---
>  8 files changed, 43 insertions(+), 27 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 5223ce8..a9355fb 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -106,7 +106,7 @@ static size_t curl_size_cb(void *ptr, size_t size, size_t 
> nmemb, void *opaque)
>      size_t realsize = size * nmemb;
>      long long fsize;
>  
> -    if(sscanf(ptr, "Content-Length: %lld", &fsize) == 1)
> +    if(sscanf(ptr, "Content-Length: %" PRId64, &fsize) == 1)
>          s->s->len = fsize;
>

fsize is long long so ll seems the right format to use.
  
>      return realsize;
> @@ -118,7 +118,7 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t 
> nmemb, void *opaque)
>      size_t realsize = size * nmemb;
>      int i;
>  
> -    dprintf("CURL: Just reading %lld bytes\n", (unsigned long long)realsize);
> +    dprintf("CURL: Just reading %" PRId64 " bytes\n", (unsigned long 
> long)realsize);
>  

Since we cast to long long, %ll is right.
Alternatively, we could use %zd and remove the cast.

>      if (!s || !s->orig_buf)
>          goto read_end;
> @@ -368,7 +368,7 @@ static int curl_open(BlockDriverState *bs, const char 
> *filename, int flags)
>          s->len = (size_t)d;
>      else if(!s->len)
>          goto out;
> -    dprintf("CURL: Size = %lld\n", (long long)s->len);
> +    dprintf("CURL: Size = %" PRId64 "\n", (long long)s->len);
>  

again, remove cast and use %zd. As it is %lld is right.

>      curl_clean_state(state);
>      curl_easy_cleanup(state->curl);
> @@ -450,8 +450,8 @@ static BlockDriverAIOCB *curl_aio_readv(BlockDriverState 
> *bs,
>      state->orig_buf = qemu_malloc(state->buf_len);
>      state->acb[0] = acb;
>  
> -    snprintf(state->range, 127, "%lld-%lld", (long long)start, (long 
> long)end);
> -    dprintf("CURL (AIO): Reading %d at %lld (%s)\n", (nb_sectors * 
> SECTOR_SIZE), start, state->range);
> +    snprintf(state->range, 127, "%" PRId64 "-%" PRId64, (long long)start, 
> (long long)end);
> +    dprintf("CURL (AIO): Reading %d at %" PRId64 " (%s)\n", (nb_sectors * 
> SECTOR_SIZE), start, state->range);
>      curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
>  
>      curl_multi_add_handle(s->multi, state->curl);

same. %z and remove cast or %lld

> diff --git a/block/qcow2.c b/block/qcow2.c
> index 6622eba..eb74564 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1153,7 +1153,7 @@ static void dump_refcounts(BlockDriverState *bs)
>          k++;
>          while (k < nb_clusters && get_refcount(bs, k) == refcount)
>              k++;
> -        printf("%lld: refcount=%d nb=%lld\n", k, refcount, k - k1);
> +        printf("%" PRId64 ": refcount=%d nb=%" PRId64 "\n", k, refcount, k - 
> k1);
>      }
>  }
>  #endif

This one seems correct: k is int64_t. But this code is in #if 0.

> diff --git a/hw/vga.c b/hw/vga.c
> index 6a1a059..dd67097 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -229,7 +229,7 @@ static void 
> vga_precise_update_retrace_info(VGACommonState *s)
>          "clocking_mode = %d\n"
>          "clock_sel = %d %d\n"
>          "dots = %d\n"
> -        "ticks/char = %lld\n"
> +        "ticks/char = %" PRId64 "\n"
>          "\n",
>          (double) get_ticks_per_sec() / (r->ticks_per_char * r->total_chars),
>          htotal_chars,

Again correct and again this is commented out code.

> diff --git a/json-lexer.c b/json-lexer.c
> index 53697c5..9d64920 100644
> --- a/json-lexer.c
> +++ b/json-lexer.c
> @@ -54,6 +54,9 @@ enum json_lexer_state {
>      IN_ESCAPE,
>      IN_ESCAPE_L,
>      IN_ESCAPE_LL,
> +    IN_ESCAPE_I,
> +    IN_ESCAPE_I6,
> +    IN_ESCAPE_I64,
>      IN_ESCAPE_DONE,
>      IN_WHITESPACE,
>      IN_OPERATOR_DONE,
> @@ -223,6 +226,18 @@ static const uint8_t json_lexer[][256] =  {
>          ['l'] = IN_ESCAPE_LL,
>      },
>  
> +    [IN_ESCAPE_I64] = {
> +        ['d'] = IN_ESCAPE_DONE,
> +    },
> +
> +    [IN_ESCAPE_I6] = {
> +        ['4'] = IN_ESCAPE_I64,
> +    },
> +
> +    [IN_ESCAPE_I] = {
> +        ['6'] = IN_ESCAPE_I6,
> +    },
> +
>      [IN_ESCAPE] = {
>          ['d'] = IN_ESCAPE_DONE,
>          ['i'] = IN_ESCAPE_DONE,
> @@ -230,6 +245,7 @@ static const uint8_t json_lexer[][256] =  {
>          ['s'] = IN_ESCAPE_DONE,
>          ['f'] = IN_ESCAPE_DONE,
>          ['l'] = IN_ESCAPE_L,
> +        ['I'] = IN_ESCAPE_I,
>      },
>  
>      /* top level rule */


Why do we want yet another tag?

OTOH, Luiz, maybe it is a mistake to use "long"
in QMP: legal values might vary between platforms.
How about we get rid of long and only use long long to mean 64
bit/int to mean 32 bit? Or even redefine "l" to mean 64 bit and "i" to
mean "32 bit.  Also, why do we allow "d" as synonym of "i"?  Keeping all
of int/long/long long around does not make sense to me though.  Finally,
don't we want unsigned values in protocol?

> diff --git a/json-parser.c b/json-parser.c
> index e04932f..a11cc53 100644
> --- a/json-parser.c
> +++ b/json-parser.c
> @@ -474,7 +474,7 @@ static QObject *parse_escape(JSONParserContext *ctxt, 
> QList **tokens, va_list *a
>          obj = QOBJECT(qint_from_int(va_arg(*ap, int)));
>      } else if (token_is_escape(token, "%ld")) {
>          obj = QOBJECT(qint_from_int(va_arg(*ap, long)));
> -    } else if (token_is_escape(token, "%lld")) {
> +    } else if (token_is_escape(token, "%" PRId64 )) {
>          obj = QOBJECT(qint_from_int(va_arg(*ap, long long)));
>      } else if (token_is_escape(token, "%s")) {
>          obj = QOBJECT(qstring_from_str(va_arg(*ap, const char *)));


I grew tired at this point.

> diff --git a/qemu-img.c b/qemu-img.c
> index 3cea8ce..8a33a3d 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -724,8 +724,8 @@ static int img_convert(int argc, char **argv)
>                      bs_offset += bs_sectors;
>                      bdrv_get_geometry(bs[bs_i], &bs_sectors);
>                      bs_num = 0;
> -                    /* printf("changing part: sector_num=%lld, "
> -                       "bs_i=%d, bs_offset=%lld, bs_sectors=%lld\n",
> +                    /* printf("changing part: sector_num=%" PRId64 ", "
> +                       "bs_i=%d, bs_offset=%" PRId64 ", bs_sectors=%" PRId64 
> "\n",
>                         sector_num, bs_i, bs_offset, bs_sectors); */
>                  }
>                  assert (bs_num < bs_sectors);
> @@ -770,8 +770,8 @@ static int img_convert(int argc, char **argv)
>                  assert (bs_i < bs_n);
>                  bs_offset += bs_sectors;
>                  bdrv_get_geometry(bs[bs_i], &bs_sectors);
> -                /* printf("changing part: sector_num=%lld, bs_i=%d, "
> -                  "bs_offset=%lld, bs_sectors=%lld\n",
> +                /* printf("changing part: sector_num=%" PRId64 ", bs_i=%d, "
> +                  "bs_offset=%" PRId64 ", bs_sectors=%" PRId64 "\n",
>                     sector_num, bs_i, bs_offset, bs_sectors); */
>              }
>  
> diff --git a/qemu-io.c b/qemu-io.c
> index b159bc9..4aa2ae5 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -108,7 +108,7 @@ print_report(const char *op, struct timeval *t, int64_t 
> offset,
>       if (!Cflag) {
>               cvtstr((double)total, s1, sizeof(s1));
>               cvtstr(tdiv((double)total, *t), s2, sizeof(s2));
> -             printf("%s %d/%d bytes at offset %lld\n",
> +             printf("%s %d/%d bytes at offset %" PRId64 "\n",
>                       op, total, count, (long long)offset);
>               printf("%s, %d ops; %s (%s/sec and %.4f ops/sec)\n",
>                       s1, cnt, ts, s2, tdiv((double)cnt, *t));
> @@ -150,7 +150,7 @@ create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, 
> int pattern)
>               }
>  
>               if (len & 0x1ff) {
> -                     printf("length argument %lld is not sector aligned\n",
> +                     printf("length argument %" PRId64 " is not sector 
> aligned\n",
>                               len);
>                       goto fail;
>               }
> @@ -398,7 +398,7 @@ read_f(int argc, char **argv)
>  
>       if (!pflag)
>               if (offset & 0x1ff) {
> -                     printf("offset %lld is not sector aligned\n",
> +                     printf("offset %" PRId64 " is not sector aligned\n",
>                               (long long)offset);
>                       return 0;
>  
> @@ -429,7 +429,7 @@ read_f(int argc, char **argv)
>               void* cmp_buf = malloc(pattern_count);
>               memset(cmp_buf, pattern, pattern_count);
>               if (memcmp(buf + pattern_offset, cmp_buf, pattern_count)) {
> -                     printf("Pattern verification failed at offset %lld, "
> +                     printf("Pattern verification failed at offset %" PRId64 
> ", "
>                               "%d bytes\n",
>                               (long long) offset + pattern_offset, 
> pattern_count);
>               }
> @@ -533,7 +533,7 @@ readv_f(int argc, char **argv)
>       optind++;
>  
>       if (offset & 0x1ff) {
> -             printf("offset %lld is not sector aligned\n",
> +             printf("offset %" PRId64 " is not sector aligned\n",
>                       (long long)offset);
>               return 0;
>       }
> @@ -554,7 +554,7 @@ readv_f(int argc, char **argv)
>               void* cmp_buf = malloc(qiov.size);
>               memset(cmp_buf, pattern, qiov.size);
>               if (memcmp(buf, cmp_buf, qiov.size)) {
> -                     printf("Pattern verification failed at offset %lld, "
> +                     printf("Pattern verification failed at offset %" PRId64 
> ", "
>                               "%zd bytes\n",
>                               (long long) offset, qiov.size);
>               }
> @@ -669,7 +669,7 @@ write_f(int argc, char **argv)
>  
>       if (!pflag) {
>               if (offset & 0x1ff) {
> -                     printf("offset %lld is not sector aligned\n",
> +                     printf("offset %" PRId64 " is not sector aligned\n",
>                               (long long)offset);
>                       return 0;
>               }
> @@ -783,7 +783,7 @@ writev_f(int argc, char **argv)
>       optind++;
>  
>       if (offset & 0x1ff) {
> -             printf("offset %lld is not sector aligned\n",
> +             printf("offset %" PRId64 " is not sector aligned\n",
>                       (long long)offset);
>               return 0;
>       }
> @@ -868,7 +868,7 @@ aio_read_done(void *opaque, int ret)
>  
>               memset(cmp_buf, ctx->pattern, ctx->qiov.size);
>               if (memcmp(ctx->buf, cmp_buf, ctx->qiov.size)) {
> -                     printf("Pattern verification failed at offset %lld, "
> +                     printf("Pattern verification failed at offset %" PRId64 
> ", "
>                               "%zd bytes\n",
>                               (long long) ctx->offset, ctx->qiov.size);
>               }
> @@ -969,7 +969,7 @@ aio_read_f(int argc, char **argv)
>       optind++;
>  
>       if (ctx->offset & 0x1ff) {
> -             printf("offset %lld is not sector aligned\n",
> +             printf("offset %" PRId64 " is not sector aligned\n",
>                       (long long)ctx->offset);
>               free(ctx);
>               return 0;
> @@ -1064,7 +1064,7 @@ aio_write_f(int argc, char **argv)
>       optind++;
>  
>       if (ctx->offset & 0x1ff) {
> -             printf("offset %lld is not sector aligned\n",
> +             printf("offset %" PRId64 " is not sector aligned\n",
>                       (long long)ctx->offset);
>               free(ctx);
>               return 0;
> @@ -1214,7 +1214,7 @@ alloc_f(int argc, char **argv)
>  
>       offset = cvtnum(argv[1]);
>       if (offset & 0x1ff) {
> -             printf("offset %lld is not sector aligned\n",
> +             printf("offset %" PRId64 " is not sector aligned\n",
>                       (long long)offset);
>               return 0;
>       }
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index d4e81ce..b61c949 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -8920,7 +8920,7 @@ void cpu_dump_statistics (CPUState *env, FILE*f,
>                          if (handler->count == 0)
>                              continue;
>                          cpu_fprintf(f, "%02x %02x %02x (%02x %04d) %16s: "
> -                                    "%016llx %lld\n",
> +                                    "%016" PRIx64 " %" PRId64 "\n",
>                                      op1, op2, op3, op1, (op3 << 5) | op2,
>                                      handler->oname,
>                                      handler->count, handler->count);
> @@ -8929,7 +8929,7 @@ void cpu_dump_statistics (CPUState *env, FILE*f,
>                      if (handler->count == 0)
>                          continue;
>                      cpu_fprintf(f, "%02x %02x    (%02x %04d) %16s: "
> -                                "%016llx %lld\n",
> +                                "%016" PRIx64 " %" PRId64 "\n",
>                                  op1, op2, op1, op2, handler->oname,
>                                  handler->count, handler->count);
>                  }
> @@ -8937,7 +8937,7 @@ void cpu_dump_statistics (CPUState *env, FILE*f,
>          } else {
>              if (handler->count == 0)
>                  continue;
> -            cpu_fprintf(f, "%02x       (%02x     ) %16s: %016llx %lld\n",
> +            cpu_fprintf(f, "%02x       (%02x     ) %16s: %016" PRIx64 " %" 
> PRId64 "\n",
>                          op1, op1, handler->oname,
>                          handler->count, handler->count);
>          }
> -- 
> 1.6.0.2.GIT
> 
>