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

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

[Herve Poussineau]

Replace %lld occurrences by PRId64.
Incidentally, this fixes use of curl on Windows, and prevents an assert
when closing Qemu.

Signed-off-by: Herve Poussineau <hpoussin@reactos.org>
---
 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;
 
     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);
 
     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);
 
     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);
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
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,
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 */
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 *)));
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

[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 <hpoussin@reactos.org>
> ---
>  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
> 
> 

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

[Luiz Capitulino]

On Mon, 25 Jan 2010 12:09:06 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Sun, Jan 24, 2010 at 09:23:41PM +0000, Herve Poussineau wrote:
> > Replace %lld occurrences by PRId64.
> > 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.

 Not that familiar with the parser (Anthony wrote it), but I guess
it will convert any int type (with the limit of int64_t) to
json int.

> 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?

 JSON doesn't support them.

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

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Mon, 25 Jan 2010 12:09:06 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
[...]
>>                                                                 Finally,
>> don't we want unsigned values in protocol?
>
>  JSON doesn't support them.

Uh, where does the RFC say that?

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

[Luiz Capitulino]

On Mon, 25 Jan 2010 15:35:53 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Mon, 25 Jan 2010 12:09:06 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> [...]
> >>                                                                 Finally,
> >> don't we want unsigned values in protocol?
> >
> >  JSON doesn't support them.
> 
> Uh, where does the RFC say that?

 I see that my comment was misleading.

 In JSON we don't have unsigned types, we have only a type
called 'number' to represent them all.

 Unsigneds should be handled correctly, except for uint64_t which
is cast to int64_t.

 Michael, does this answer your question? Is there any
issue with the handling of unsigneds I'm not aware about?

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

[Michael S. Tsirkin]

On Mon, Jan 25, 2010 at 01:27:19PM -0200, Luiz Capitulino wrote:
> On Mon, 25 Jan 2010 15:35:53 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
> 
> > Luiz Capitulino <lcapitulino@redhat.com> writes:
> > 
> > > On Mon, 25 Jan 2010 12:09:06 +0200
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > [...]
> > >>                                                                 Finally,
> > >> don't we want unsigned values in protocol?
> > >
> > >  JSON doesn't support them.
> > 
> > Uh, where does the RFC say that?
> 
>  I see that my comment was misleading.
> 
>  In JSON we don't have unsigned types, we have only a type
> called 'number' to represent them all.
> 
>  Unsigneds should be handled correctly, except for uint64_t which
> is cast to int64_t.
> 
>  Michael, does this answer your question?
> Is there any
> issue with the handling of unsigneds I'm not aware about?

The issue I see isn't related to unsigned.  Apparently we currently
accept values such as 'a' as valid strings. Since this is not valid json
we probably should reject it just in case we will want to switch to
another json library, otherwise clients might come to depend on
non-standard behaviour.

-- 
MST

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

[Luiz Capitulino]

On Mon, 25 Jan 2010 17:38:38 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jan 25, 2010 at 01:27:19PM -0200, Luiz Capitulino wrote:
> > On Mon, 25 Jan 2010 15:35:53 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> > 
> > > Luiz Capitulino <lcapitulino@redhat.com> writes:
> > > 
> > > > On Mon, 25 Jan 2010 12:09:06 +0200
> > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > [...]
> > > >>                                                                 Finally,
> > > >> don't we want unsigned values in protocol?
> > > >
> > > >  JSON doesn't support them.
> > > 
> > > Uh, where does the RFC say that?
> > 
> >  I see that my comment was misleading.
> > 
> >  In JSON we don't have unsigned types, we have only a type
> > called 'number' to represent them all.
> > 
> >  Unsigneds should be handled correctly, except for uint64_t which
> > is cast to int64_t.
> > 
> >  Michael, does this answer your question?
> > Is there any
> > issue with the handling of unsigneds I'm not aware about?
> 
> The issue I see isn't related to unsigned.  Apparently we currently
> accept values such as 'a' as valid strings. Since this is not valid json
> we probably should reject it just in case we will want to switch to
> another json library, otherwise clients might come to depend on
> non-standard behaviour.

 This extension is only used internally by QEMU and we find it
very convenient otherwise we would have to escape strings in
dicts and lists, which is error prone and time consuming.

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

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Mon, 25 Jan 2010 17:38:38 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
[...]
>> The issue I see isn't related to unsigned.  Apparently we currently
>> accept values such as 'a' as valid strings. Since this is not valid json
>> we probably should reject it just in case we will want to switch to
>> another json library, otherwise clients might come to depend on
>> non-standard behaviour.
>
>  This extension is only used internally by QEMU

Let me elaborate: when a QEMP client sends us 'a' over the wire, the
parser rejects that as an error.  At least that's what we've been
promised when the extension was discussed.

>                                                 and we find it
> very convenient otherwise we would have to escape strings in
> dicts and lists, which is error prone and time consuming.

I doubt it would be error prone, but it would sure be annoying and hard
to read.  The readability argument is what convinced me.

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

[Anthony Liguori]

On 01/26/2010 06:37 AM, Markus Armbruster wrote:
>>   This extension is only used internally by QEMU
>>      
> Let me elaborate: when a QEMP client sends us 'a' over the wire, the
> parser rejects that as an error.  At least that's what we've been
> promised when the extension was discussed.
>    

No, that's never been the case.  I don't see the point.  JSON allows 
it.  If a client comes to depend on it, so what?

Regards,

Anthony Liguori

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

[Michael S. Tsirkin]

On Tue, Jan 26, 2010 at 06:56:10AM -0600, Anthony Liguori wrote:
> On 01/26/2010 06:37 AM, Markus Armbruster wrote:
>>>   This extension is only used internally by QEMU
>>>      
>> Let me elaborate: when a QEMP client sends us 'a' over the wire, the
>> parser rejects that as an error.  At least that's what we've been
>> promised when the extension was discussed.
>>    
>
> No, that's never been the case.  I don't see the point.  JSON allows it.  
> If a client comes to depend on it, so what?
>
> Regards,
>
> Anthony Liguori


Then we'll have to support it forever. Asking clients to only depend on
valid JSON will make sure we can use json library in the future, as well
as allow easier debugging etc.

-- 
MST

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

[Anthony Liguori]

On 01/26/2010 06:56 AM, Michael S. Tsirkin wrote:
> Then we'll have to support it forever. Asking clients to only depend on
> valid JSON will make sure we can use json library in the future, as well
> as allow easier debugging etc.
>    

As I mentioned in IRC, I'm not opposed to making this feature of the 
parser not exposed when dealing with external clients.

But I do believe that we're going to have to extend JSON down the road.  
This particular extension is unimportant so I don't mind limiting it's 
visibility.

Regards,

Anthony Liguori

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

[Michael S. Tsirkin]

On Tue, Jan 26, 2010 at 07:05:01AM -0600, Anthony Liguori wrote:
> On 01/26/2010 06:56 AM, Michael S. Tsirkin wrote:
>> Then we'll have to support it forever. Asking clients to only depend on
>> valid JSON will make sure we can use json library in the future, as well
>> as allow easier debugging etc.
>>    
>
> As I mentioned in IRC, I'm not opposed to making this feature of the  
> parser not exposed when dealing with external clients.
>
> But I do believe that we're going to have to extend JSON down the road.   
> This particular extension is unimportant so I don't mind limiting it's  
> visibility.

I agree.

> Regards,
>
> Anthony Liguori

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

[Anthony Liguori]

On 01/26/2010 05:43 AM, Luiz Capitulino wrote:
>> The issue I see isn't related to unsigned.  Apparently we currently
>> accept values such as 'a' as valid strings. Since this is not valid json
>> we probably should reject it just in case we will want to switch to
>> another json library, otherwise clients might come to depend on
>> non-standard behaviour.
>>      
>   This extension is only used internally by QEMU and we find it
> very convenient otherwise we would have to escape strings in
> dicts and lists, which is error prone and time consuming.
>    

Actually, I was reading the JSON RFC last night and came across:

   "A JSON parser transforms a JSON text into another representation. A
    JSON parser MUST accept all texts that conform to the JSON grammar.
    A JSON parser MAY accept non-JSON forms or extensions."

So we are fully JSON compliant in our current implementation.

Regards,

Anthony Liguori

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

[Michael S. Tsirkin]

On Tue, Jan 26, 2010 at 06:47:12AM -0600, Anthony Liguori wrote:
> On 01/26/2010 05:43 AM, Luiz Capitulino wrote:
>>> The issue I see isn't related to unsigned.  Apparently we currently
>>> accept values such as 'a' as valid strings. Since this is not valid json
>>> we probably should reject it just in case we will want to switch to
>>> another json library, otherwise clients might come to depend on
>>> non-standard behaviour.
>>>      
>>   This extension is only used internally by QEMU and we find it
>> very convenient otherwise we would have to escape strings in
>> dicts and lists, which is error prone and time consuming.
>>    
>
> Actually, I was reading the JSON RFC last night and came across:
>
>   "A JSON parser transforms a JSON text into another representation. A
>    JSON parser MUST accept all texts that conform to the JSON grammar.
>    A JSON parser MAY accept non-JSON forms or extensions."
>
> So we are fully JSON compliant in our current implementation.
>
> Regards,
>
> Anthony Liguori

Yes, I agree we are comnpliant.
But I also think we should be strict and reject non-JSON
input just so that clients do not come to depend on it.

-- 
MST

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

[Anthony Liguori]

On 01/26/2010 06:46 AM, Michael S. Tsirkin wrote:
> Yes, I agree we are comnpliant.
> But I also think we should be strict and reject non-JSON
> input just so that clients do not come to depend on it.
>    

If we can make JSON better while preserving compatibility and adhering 
to the spec, why wouldn't we?

For instance, at some point in time, we're going to do have to do 
something about floating point representation.  We have the ability to 
negotiate these capabilities at run-time.

Regards,

Anthony Liguori

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

[Michael S. Tsirkin]

On Tue, Jan 26, 2010 at 06:58:32AM -0600, Anthony Liguori wrote:
> On 01/26/2010 06:46 AM, Michael S. Tsirkin wrote:
>> Yes, I agree we are comnpliant.
>> But I also think we should be strict and reject non-JSON
>> input just so that clients do not come to depend on it.
>>    
>
> If we can make JSON better while preserving compatibility and adhering  
> to the spec, why wouldn't we?

Adding '' seems very little gain. The pain point wouild be
supporting multiple syntax variants, and inability to use
external tools to parse such traffic.

> For instance, at some point in time, we're going to do have to do  
> something about floating point representation.

What's the issue? There's '.' and there's 'e' ...
And maybe we won't need floating point ever ...

> We have the ability to  negotiate these capabilities at run-time.
>
> Regards,
>
> Anthony Liguori

If there's an important capability this might make sense.

-- 
MST

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

[Anthony Liguori]

On 01/26/2010 07:01 AM, Michael S. Tsirkin wrote:
>> For instance, at some point in time, we're going to do have to do
>> something about floating point representation.
>>      
> What's the issue? There's '.' and there's 'e' ...
> And maybe we won't need floating point ever ...
>    

You cannot represent an IEEE754 floating point value because it lacks 
important representations like NaN and infinity.

Regards,

Anthony Liguori

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

[Daniel P. Berrange]

On Tue, Jan 26, 2010 at 06:58:32AM -0600, Anthony Liguori wrote:
> On 01/26/2010 06:46 AM, Michael S. Tsirkin wrote:
> >Yes, I agree we are comnpliant.
> >But I also think we should be strict and reject non-JSON
> >input just so that clients do not come to depend on it.
> >   
> 
> If we can make JSON better while preserving compatibility and adhering 
> to the spec, why wouldn't we?
> 
> For instance, at some point in time, we're going to do have to do 
> something about floating point representation.  We have the ability to 
> negotiate these capabilities at run-time.

Even if we can negotiate extensions at the protocol level, we need to be
careful about how we actually use them. The client is likely going to be
using whatever standard JSON client comes with their language/environment
and will not neccessarily have ability to change that to make use of the
QEMU specific extension. We don't want to end up with QEMU having a nice
JSON extension for some core feature, but none of the clients being able
to use it in practice.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

[Anthony Liguori]

On 01/26/2010 07:08 AM, Daniel P. Berrange wrote:
> On Tue, Jan 26, 2010 at 06:58:32AM -0600, Anthony Liguori wrote:
>    
>> On 01/26/2010 06:46 AM, Michael S. Tsirkin wrote:
>>      
>>> Yes, I agree we are comnpliant.
>>> But I also think we should be strict and reject non-JSON
>>> input just so that clients do not come to depend on it.
>>>
>>>        
>> If we can make JSON better while preserving compatibility and adhering
>> to the spec, why wouldn't we?
>>
>> For instance, at some point in time, we're going to do have to do
>> something about floating point representation.  We have the ability to
>> negotiate these capabilities at run-time.
>>      
> Even if we can negotiate extensions at the protocol level, we need to be
> careful about how we actually use them. The client is likely going to be
> using whatever standard JSON client comes with their language/environment
> and will not neccessarily have ability to change that to make use of the
> QEMU specific extension. We don't want to end up with QEMU having a nice
> JSON extension for some core feature, but none of the clients being able
> to use it in practice.
>    

Agreed.

Regards,

Anthony Liguori

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

[Avi Kivity]

On 01/26/2010 02:47 PM, Anthony Liguori wrote:
> On 01/26/2010 05:43 AM, Luiz Capitulino wrote:
>>> The issue I see isn't related to unsigned.  Apparently we currently
>>> accept values such as 'a' as valid strings. Since this is not valid 
>>> json
>>> we probably should reject it just in case we will want to switch to
>>> another json library, otherwise clients might come to depend on
>>> non-standard behaviour.
>>   This extension is only used internally by QEMU and we find it
>> very convenient otherwise we would have to escape strings in
>> dicts and lists, which is error prone and time consuming.
>
> Actually, I was reading the JSON RFC last night and came across:
>
>   "A JSON parser transforms a JSON text into another representation. A
>    JSON parser MUST accept all texts that conform to the JSON grammar.
>    A JSON parser MAY accept non-JSON forms or extensions."
>
> So we are fully JSON compliant in our current implementation.
>

The risk is that if we support a private extension (like '') and then 
json is officially extended to support a conflicting or similar syntax 
with a different meaning, then we cannot advance to the next revision of 
json without breaking compatibility.

In the case of '', the odds of a such a clash are very low, but 
nevertheless I think we should refrain from doing so.  Being strict is good.

-- 
error compiling committee.c: too many arguments to function

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

[Anthony Liguori]

On 01/26/2010 07:55 AM, Avi Kivity wrote:
> The risk is that if we support a private extension (like '') and then 
> json is officially extended to support a conflicting or similar syntax 
> with a different meaning, then we cannot advance to the next revision 
> of json without breaking compatibility.

The paragraph I quoted from the RFC seems to suggest that the authors of 
JSON boxed themselves in with respect to extending JSON.  The reason 
being that a conforming implementation is given free reign to extend 
with "non-JSON forms or extensions".  That would seem to prevent any 
extension.

Keep in mind, JSON is a proper subset of ECMAScript which means the 
likelihood of extension going outside of ECMAScript would be extremely 
unlikely.  I don't expect JSON is ever going to change.

Regards,

Anthony Liguori

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

[Avi Kivity]

On 01/26/2010 04:05 PM, Anthony Liguori wrote:
> On 01/26/2010 07:55 AM, Avi Kivity wrote:
>> The risk is that if we support a private extension (like '') and then 
>> json is officially extended to support a conflicting or similar 
>> syntax with a different meaning, then we cannot advance to the next 
>> revision of json without breaking compatibility.
>
> The paragraph I quoted from the RFC seems to suggest that the authors 
> of JSON boxed themselves in with respect to extending JSON.  The 
> reason being that a conforming implementation is given free reign to 
> extend with "non-JSON forms or extensions".  That would seem to 
> prevent any extension.

A json generator is required to generate conforming text.  So there are 
three choices:

- reject 's
- unofficially accept 's, nonconforming generators break if json 
changes, nor our problem
- officially accept 's, look stupid when json changes

>
> Keep in mind, JSON is a proper subset of ECMAScript which means the 
> likelihood of extension going outside of ECMAScript would be extremely 
> unlikely.  I don't expect JSON is ever going to change.

Who knows?  Let's not take unnecessary risks.

-- 
error compiling committee.c: too many arguments to function

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

[Anthony Liguori]

On 01/26/2010 08:12 AM, Avi Kivity wrote:
> On 01/26/2010 04:05 PM, Anthony Liguori wrote:
>> On 01/26/2010 07:55 AM, Avi Kivity wrote:
>>> The risk is that if we support a private extension (like '') and 
>>> then json is officially extended to support a conflicting or similar 
>>> syntax with a different meaning, then we cannot advance to the next 
>>> revision of json without breaking compatibility.
>>
>> The paragraph I quoted from the RFC seems to suggest that the authors 
>> of JSON boxed themselves in with respect to extending JSON.  The 
>> reason being that a conforming implementation is given free reign to 
>> extend with "non-JSON forms or extensions".  That would seem to 
>> prevent any extension.
>
> A json generator is required to generate conforming text.  So there 
> are three choices:
>
> - reject 's
> - unofficially accept 's, nonconforming generators break if json 
> changes, nor our problem
> - officially accept 's, look stupid when json changes
>
>>
>> Keep in mind, JSON is a proper subset of ECMAScript which means the 
>> likelihood of extension going outside of ECMAScript would be 
>> extremely unlikely.  I don't expect JSON is ever going to change.
>
> Who knows?  Let's not take unnecessary risks.

Keep in mind, I've already agreed to not allow '' strings for external 
JSON.  The only reason the thread's still alive is because we like to 
argue apparently :-)  Single quoted strings are not sufficiently useful 
to warrant taking any risks here.

Regards,

Anthony Liguori

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

[Stefan Weil]

Michael S. Tsirkin schrieb:
> 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.
>

size_t => %zu, ssize_t => %zd might be better.

And none of them works on win32, so using them
there can result in a crash:

    size_t st = 4711;
    fprintf(stderr, "st=%zu, %s\n", st, "test");

printf functions on win32 don't know %z.
They run

    fprintf(stderr, "st=zu, %s\n", st, "test");

which results in an memory access fault when printf
wants to read the memory at address 0x4711.

Regards,
Stefan Weil

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

[Michael S. Tsirkin]

On Mon, Jan 25, 2010 at 06:32:06PM +0100, Stefan Weil wrote:
> Michael S. Tsirkin schrieb:
> > 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.
> >
> 
> size_t => %zu, ssize_t => %zd might be better.
> 
> And none of them works on win32, so using them
> there can result in a crash:
> 
>     size_t st = 4711;
>     fprintf(stderr, "st=%zu, %s\n", st, "test");
> 
> printf functions on win32 don't know %z.
> They run
> 
>     fprintf(stderr, "st=zu, %s\n", st, "test");
> 
> which results in an memory access fault when printf
> wants to read the memory at address 0x4711.
> 
> Regards,
> Stefan Weil

Let's just implement a compliant printf?

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

[Stefan Weil]

Michael S. Tsirkin schrieb:
> On Mon, Jan 25, 2010 at 06:32:06PM +0100, Stefan Weil wrote:
>> Michael S. Tsirkin schrieb:
>>> 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.
>>>
>> size_t => %zu, ssize_t => %zd might be better.
>>
>> And none of them works on win32, so using them
>> there can result in a crash:
>>
>> size_t st = 4711;
>> fprintf(stderr, "st=%zu, %s\n", st, "test");
>>
>> printf functions on win32 don't know %z.
>> They run
>>
>> fprintf(stderr, "st=zu, %s\n", st, "test");
>>
>> which results in an memory access fault when printf
>> wants to read the memory at address 0x4711.
>>
>> Regards,
>> Stefan Weil
>
> Let's just implement a compliant printf?

Or format the harddisk and install linux?
Maybe that would be the better option :-)

Of course you can add a printf to qemu, or to mingw32.
No need to implement it - there are lots of good free
implementations.

The mingw developers are aware of the problem
(http://www.mail-archive.com/mingw-w64-public@lists.sourceforge.net/msg00416.html).

If there is an easy solution, they will fix the problem.

I don't think the problem can be fixed easy:
there is not only printf but a lot of functions which use
format strings. They are implemented in msvcrt.dll.
Replacing single functions in a dll is difficult.
Telling code which printf in which dll is the correct
one is difficult, too.

There are easy solutions for QEMU: type cast
size_t values to unsigned or uint32_t in printf
or use a new macro (for example PRIsize) in
format strings. That macro would be different for
mingw32 and standard conforming systems.

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

[Michael S. Tsirkin]

On Mon, Jan 25, 2010 at 08:23:22PM +0100, Stefan Weil wrote:
> Michael S. Tsirkin schrieb:
> > On Mon, Jan 25, 2010 at 06:32:06PM +0100, Stefan Weil wrote:
> >> Michael S. Tsirkin schrieb:
> >>> 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.
> >>>
> >> size_t => %zu, ssize_t => %zd might be better.
> >>
> >> And none of them works on win32, so using them
> >> there can result in a crash:
> >>
> >> size_t st = 4711;
> >> fprintf(stderr, "st=%zu, %s\n", st, "test");
> >>
> >> printf functions on win32 don't know %z.
> >> They run
> >>
> >> fprintf(stderr, "st=zu, %s\n", st, "test");
> >>
> >> which results in an memory access fault when printf
> >> wants to read the memory at address 0x4711.
> >>
> >> Regards,
> >> Stefan Weil
> >
> > Let's just implement a compliant printf?
> 
> Or format the harddisk and install linux?
> Maybe that would be the better option :-)
> 
> Of course you can add a printf to qemu, or to mingw32.
> No need to implement it - there are lots of good free
> implementations.
> 
> The mingw developers are aware of the problem
> (http://www.mail-archive.com/mingw-w64-public@lists.sourceforge.net/msg00416.html).
> 
> If there is an easy solution, they will fix the problem.
> 
> I don't think the problem can be fixed easy:
> there is not only printf but a lot of functions which use
> format strings. They are implemented in msvcrt.dll.
> Replacing single functions in a dll is difficult.
> Telling code which printf in which dll is the correct
> one is difficult, too.
> 
> There are easy solutions for QEMU: type cast
> size_t values to unsigned or uint32_t in printf
> or use a new macro (for example PRIsize) in
> format strings. That macro would be different for
> mingw32 and standard conforming systems.


The link above suggests adding
-D__USE_MINGW_ANSI_STDIO=1
why don't we do just do this with mingw?

People should also build with -Werror, then
it would be a build error not a crash.

-- 
MST

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

[Michael S. Tsirkin]

On Mon, Jan 25, 2010 at 09:32:05PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 25, 2010 at 08:23:22PM +0100, Stefan Weil wrote:
> > Michael S. Tsirkin schrieb:
> > > On Mon, Jan 25, 2010 at 06:32:06PM +0100, Stefan Weil wrote:
> > >> Michael S. Tsirkin schrieb:
> > >>> 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.
> > >>>
> > >> size_t => %zu, ssize_t => %zd might be better.
> > >>
> > >> And none of them works on win32, so using them
> > >> there can result in a crash:
> > >>
> > >> size_t st = 4711;
> > >> fprintf(stderr, "st=%zu, %s\n", st, "test");
> > >>
> > >> printf functions on win32 don't know %z.
> > >> They run
> > >>
> > >> fprintf(stderr, "st=zu, %s\n", st, "test");
> > >>
> > >> which results in an memory access fault when printf
> > >> wants to read the memory at address 0x4711.
> > >>
> > >> Regards,
> > >> Stefan Weil
> > >
> > > Let's just implement a compliant printf?
> > 
> > Or format the harddisk and install linux?
> > Maybe that would be the better option :-)
> > 
> > Of course you can add a printf to qemu, or to mingw32.
> > No need to implement it - there are lots of good free
> > implementations.
> > 
> > The mingw developers are aware of the problem
> > (http://www.mail-archive.com/mingw-w64-public@lists.sourceforge.net/msg00416.html).
> > 
> > If there is an easy solution, they will fix the problem.
> > 
> > I don't think the problem can be fixed easy:
> > there is not only printf but a lot of functions which use
> > format strings. They are implemented in msvcrt.dll.
> > Replacing single functions in a dll is difficult.
> > Telling code which printf in which dll is the correct
> > one is difficult, too.
> > 
> > There are easy solutions for QEMU: type cast
> > size_t values to unsigned or uint32_t in printf
> > or use a new macro (for example PRIsize) in
> > format strings. That macro would be different for
> > mingw32 and standard conforming systems.
> 
> 
> The link above suggests adding
> -D__USE_MINGW_ANSI_STDIO=1
> why don't we do just do this with mingw?

Looking a bit lower, maybe just -std=c99.

> People should also build with -Werror, then
> it would be a build error not a crash.
> 
> -- 
> MST

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

[Daniel P. Berrange]

On Mon, Jan 25, 2010 at 07:47:56PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 25, 2010 at 06:32:06PM +0100, Stefan Weil wrote:
> > Michael S. Tsirkin schrieb:
> > > 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.
> > >
> > 
> > size_t => %zu, ssize_t => %zd might be better.
> > 
> > And none of them works on win32, so using them
> > there can result in a crash:
> > 
> >     size_t st = 4711;
> >     fprintf(stderr, "st=%zu, %s\n", st, "test");
> > 
> > printf functions on win32 don't know %z.
> > They run
> > 
> >     fprintf(stderr, "st=zu, %s\n", st, "test");
> > 
> > which results in an memory access fault when printf
> > wants to read the memory at address 0x4711.
> > 
> > Regards,
> > Stefan Weil
> 
> Let's just implement a compliant printf?

It is a shame that QEMU isn't using GNULIB, since that comes with a nice
portable replacement printf() implementation that is transparently used
on any broken platform. Beyond just Win32, a great many UNIX have broken
printf() in one form or another too:

  http://www.gnu.org/software/hello/manual/gnulib/printf.html

I don't know how easy it would be to extract the gnulib printf() impl on
its own and use that in QEMU without the entire gnulib infrastructure

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

[Stefan Weil]

Stefan Weil schrieb:
> Michael S. Tsirkin schrieb:
>> 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.
>>
>
> size_t => %zu, ssize_t => %zd might be better.
>
> And none of them works on win32, so using them
> there can result in a crash:
>
> size_t st = 4711;
> fprintf(stderr, "st=%zu, %s\n", st, "test");
>
> printf functions on win32 don't know %z.
> They run
>
> fprintf(stderr, "st=zu, %s\n", st, "test");
>
> which results in an memory access fault when printf
> wants to read the memory at address 0x4711.
>
> Regards,
> Stefan Weil
>
>

Hi,

I just read this which could explain crashes with %lld:

/* MSVCRT supports additional length specifiers for "printf". (In
fact, it does not support some of the C99 specifiers, like
"ll". However, we do not presently have a mechanism for disabling
a specifier.) */

A short test:
    long long ll = 0;
    printf("ll=%lld, string=%s\n", ll, "test");

ll=0, string=(null)

=> You can crash QEMU for win32 with %lld.

Regards,
Stefan

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

[Anthony Liguori]

On 01/24/2010 03:23 PM, Herve Poussineau wrote:
> Replace %lld occurrences by PRId64.
> Incidentally, this fixes use of curl on Windows, and prevents an assert
> when closing Qemu.
>    

The JSON bits need to be a separate patch.  The changes to qemu-io.c 
break the build on 64-bit too.

Regards,

Anthony Liguori