Re: [PATCH spice-server 1/33 v2] Use proper format strings for spice_log

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 02, 2019 at 02:56:43PM +0000, Frediano Ziglio wrote:
> Formatting string should be compatible with GLib.
> GLib uses formatting types compatible with GNU.
> For Linux this is not an issue as both systems (like a printf) and
> GLib one uses the same formatting type.  However on Windows they
> differs potentially causing issues.
> This is also make worse as GLib 2.58 changed format attribute from
> __printf__ to gnu_printf (Microsoft compatibility formats like %I64d
> are still supported but you'll get warnings using GCC/Clang
> compilers).
> 
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
>  server/char-device.c         |  3 ++-
>  server/gstreamer-encoder.c   |  4 ++--
>  server/main-channel-client.c |  6 +++---
>  server/memslot.c             |  2 +-
>  server/mjpeg-encoder.c       |  6 ++++--
>  server/red-channel.c         |  6 ++++--
>  server/red-client.c          |  6 ++++--
>  server/red-replay-qxl.c      | 11 ++++++-----
>  server/reds.c                |  4 ++--
>  subprojects/spice-common     |  2 +-
>  10 files changed, 29 insertions(+), 21 deletions(-)
> 
> Changes since v1:
> - update due to GLib changes and compatibility to use GLib format
>   types.
> 
> diff --git a/server/char-device.c b/server/char-device.c
> index 64b41a94..c00e96ef 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -786,7 +786,8 @@ void red_char_device_client_remove(RedCharDevice *dev,
>      }
>  
>      if (dev->priv->clients == NULL) {
> -        spice_debug("client removed, memory pool will be freed (%"PRIu64" bytes)", dev->priv->cur_pool_size);
> +        spice_debug("client removed, memory pool will be freed (%"G_GUINT64_FORMAT" bytes)",
> +                    dev->priv->cur_pool_size);
>          write_buffers_queue_free(&dev->priv->write_bufs_pool);
>          dev->priv->cur_pool_size = 0;
>      }
> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> index 04f0c02f..972c86e6 100644
> --- a/server/gstreamer-encoder.c
> +++ b/server/gstreamer-encoder.c
> @@ -1083,7 +1083,7 @@ static void set_gstenc_bitrate(SpiceGstEncoder *encoder)
>          spice_warning("the %s property has an unsupported type %zu",
>                        prop, param->value_type);
>      }
> -    spice_debug("setting the GStreamer %s to %"PRIu64, prop, gst_bit_rate);
> +    spice_debug("setting the GStreamer %s to %"G_GUINT64_FORMAT, prop, gst_bit_rate);
>  }
>  
>  /* A helper for spice_gst_encoder_encode_frame() */
> diff --git a/server/main-channel-client.c b/server/main-channel-client.c
> index 54be9934..3b6ae269 100644
> --- a/server/main-channel-client.c
> +++ b/server/main-channel-client.c
> @@ -529,8 +529,8 @@ void main_channel_client_handle_pong(MainChannelClient *mcc, SpiceMsgPing *ping,
>          if (roundtrip <= mcc->priv->latency) {
>              // probably high load on client or server result with incorrect values
>              red_channel_debug(red_channel_client_get_channel(rcc),
> -                              "net test: invalid values, latency %" PRIu64
> -                              " roundtrip %" PRIu64 ". assuming high"
> +                              "net test: invalid values, latency %" G_GUINT64_FORMAT
> +                              " roundtrip %"G_GUINT64_FORMAT". assuming high"
>                                "bandwidth", mcc->priv->latency, roundtrip);
>              mcc->priv->latency = 0;
>              mcc->priv->net_test_stage = NET_TEST_STAGE_INVALID;
> @@ -542,7 +542,7 @@ void main_channel_client_handle_pong(MainChannelClient *mcc, SpiceMsgPing *ping,
>              / (roundtrip - mcc->priv->latency);
>          mcc->priv->net_test_stage = NET_TEST_STAGE_COMPLETE;
>          red_channel_debug(red_channel_client_get_channel(rcc),
> -                          "net test: latency %f ms, bitrate %"PRIu64" bps (%f Mbps)%s",
> +                          "net test: latency %f ms, bitrate %"G_GUINT64_FORMAT" bps (%f Mbps)%s",
>                            (double)mcc->priv->latency / 1000,
>                            mcc->priv->bitrate_per_sec,
>                            (double)mcc->priv->bitrate_per_sec / 1024 / 1024,
> diff --git a/server/memslot.c b/server/memslot.c
> index c2931321..dae04d2f 100644
> --- a/server/memslot.c
> +++ b/server/memslot.c
> @@ -105,7 +105,7 @@ void *memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size
>      slot_id = memslot_get_id(info, addr);
>      if (slot_id > info->num_memslots) {
>          print_memslots(info);
> -        spice_critical("slot_id %d too big, addr=%" PRIx64, slot_id, addr);
> +        spice_critical("slot_id %d too big, addr=%" G_GINT64_MODIFIER "x", slot_id, addr);
>          return NULL;
>      }
>  
> diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c
> index d4b5c6fc..d64fcfe0 100644
> --- a/server/mjpeg-encoder.c
> +++ b/server/mjpeg-encoder.c
> @@ -614,7 +614,8 @@ static void mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
>  
>      spice_debug("cur-fps=%u new-fps=%u (new/old=%.2f) |"
>                  "bit-rate=%.2f (Mbps) latency=%u (ms) quality=%d |"
> -                " new-size-avg %"PRIu64" , base-size %"PRIu64", (new/old=%.2f) ",
> +                " new-size-avg %"G_GUINT64_FORMAT" ,"
> +                " base-size %"G_GUINT64_FORMAT", (new/old=%.2f) ",
>                  rate_control->fps, new_fps, ((double)new_fps)/rate_control->fps,
>                  ((double)rate_control->byte_rate*8)/1024/1024,
>                  latency,
> @@ -676,7 +677,8 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder, uint64_t now)
>  
>          avg_fps = ((double)rate_control->adjusted_fps_num_frames * MSEC_PER_SEC) /
>                    adjusted_fps_time_passed;
> -        spice_debug("#frames-adjust=%"PRIu64" #adjust-time=%"PRIu64" avg-fps=%.2f",
> +        spice_debug("#frames-adjust=%"G_GUINT64_FORMAT
> +                    " #adjust-time=%"G_GUINT64_FORMAT" avg-fps=%.2f",
>                      rate_control->adjusted_fps_num_frames, adjusted_fps_time_passed, avg_fps);
>          spice_debug("defined=%u old-adjusted=%.2f", rate_control->fps, rate_control->adjusted_fps);
>          fps_ratio = avg_fps / rate_control->fps;
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 448b690e..333a3e58 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -190,7 +190,7 @@ red_channel_constructed(GObject *object)
>  {
>      RedChannel *self = RED_CHANNEL(object);
>  
> -    red_channel_debug(self, "thread_id 0x%lx", self->priv->thread_id);
> +    red_channel_debug(self, "thread_id 0x%" G_GSIZE_MODIFIER "x", self->priv->thread_id);
>  
>      RedChannelClass *klass = RED_CHANNEL_GET_CLASS(self);
>  
> @@ -468,7 +468,9 @@ void red_channel_remove_client(RedChannel *channel, RedChannelClient *rcc)
>      g_return_if_fail(channel == red_channel_client_get_channel(rcc));
>  
>      if (!pthread_equal(pthread_self(), channel->priv->thread_id)) {
> -        red_channel_warning(channel, "channel->thread_id (0x%lx) != pthread_self (0x%lx)."
> +        red_channel_warning(channel,
> +                            "channel->thread_id (0x%" G_GSIZE_MODIFIER "x) != "
> +                            "pthread_self (0x%" G_GSIZE_MODIFIER "x)."
>                              "If one of the threads is != io-thread && != vcpu-thread, "
>                              "this might be a BUG",
>                              channel->priv->thread_id, pthread_self());
> diff --git a/server/red-client.c b/server/red-client.c
> index 7d14bd19..98838357 100644
> --- a/server/red-client.c
> +++ b/server/red-client.c
> @@ -176,7 +176,8 @@ void red_client_migrate(RedClient *client)
>      RedChannel *channel;
>  
>      if (!pthread_equal(pthread_self(), client->thread_id)) {
> -        spice_warning("client->thread_id (0x%lx) != pthread_self (0x%lx)."
> +        spice_warning("client->thread_id (0x%" G_GSIZE_MODIFIER "x) != "
> +                      "pthread_self (0x%" G_GSIZE_MODIFIER "x)."
>                        "If one of the threads is != io-thread && != vcpu-thread,"
>                        " this might be a BUG",
>                        client->thread_id, pthread_self());
> @@ -194,7 +195,8 @@ void red_client_destroy(RedClient *client)
>      RedChannelClient *rcc;
>  
>      if (!pthread_equal(pthread_self(), client->thread_id)) {
> -        spice_warning("client->thread_id (0x%lx) != pthread_self (0x%lx)."
> +        spice_warning("client->thread_id (0x%" G_GSIZE_MODIFIER "x) != "
> +                      "pthread_self (0x%" G_GSIZE_MODIFIER "x)."
>                        "If one of the threads is != io-thread && != vcpu-thread,"
>                        " this might be a BUG",
>                        client->thread_id,
> diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> index 6958a495..4884e97e 100644
> --- a/server/red-replay-qxl.c
> +++ b/server/red-replay-qxl.c
> @@ -227,7 +227,7 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
>      uint8_t *zlib_buffer;
>      z_stream strm;
>  
> -    snprintf(template, sizeof(template), "binary %%d %s %%ld:%%n", prefix);
> +    snprintf(template, sizeof(template), "binary %%d %s %%" PRIdPTR ":%%n", prefix);

This one introduces the use of PRIdPTR, might be worth moving it to a
separate commit.

>      replay_fscanf_check(replay, template, &with_zlib, size, &replay->end_pos);
>      if (replay->error) {
>          return REPLAY_ERROR;
> @@ -266,7 +266,8 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
>              exit(1);
>          }
>          if ((ret = inflate(&strm, Z_NO_FLUSH)) != Z_STREAM_END) {
> -            spice_error("inflate error %d (disc: %ld)", ret, *size - strm.total_out);
> +            spice_error("inflate error %d (disc: %" G_GSSIZE_FORMAT ")",
> +                        ret, *size - strm.total_out);
>              if (ret == Z_DATA_ERROR) {
>                  /* last operation may be wrong. since we do the recording
>                   * in red_worker, when there is a shutdown from the vcpu/io thread
> @@ -475,7 +476,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
>          } else {
>              size = red_replay_data_chunks(replay, "bitmap.data", (uint8_t**)&qxl->bitmap.data, 0);
>              if (size != bitmap_size) {
> -                g_warning("bad image, %" PRIuPTR " != %" PRIuPTR, size, bitmap_size);
> +                g_warning("bad image, %" G_GSIZE_FORMAT " != %" G_GSIZE_FORMAT, size, bitmap_size);
>                  return NULL;
>              }
>          }
> @@ -1137,7 +1138,7 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
>          if ((qxl->flags & QXL_SURF_FLAG_KEEP_DATA) != 0) {
>              read_binary(replay, "data", &read_size, (uint8_t**)&qxl->u.surface_create.data, 0);
>              if (read_size != size) {
> -                g_warning("mismatch %" PRIuPTR " != %" PRIuPTR, size, read_size);
> +                g_warning("mismatch %" G_GSIZE_FORMAT " != %" G_GSIZE_FORMAT, size, read_size);
>              }
>          } else {
>              qxl->u.surface_create.data = QXLPHYSICAL_FROM_PTR(replay_malloc(replay, size));
> @@ -1321,7 +1322,7 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
>      cmd = replay_malloc0(replay, sizeof(QXLCommandExt));
>      cmd->cmd.type = type;
>      cmd->group_id = 0;
> -    spice_debug("command %"SCNu64", %d", timestamp, cmd->cmd.type);
> +    spice_debug("command %"G_GUINT64_FORMAT", %d", timestamp, cmd->cmd.type);
>      switch (cmd->cmd.type) {
>      case QXL_CMD_DRAW:
>          cmd->flags = 0;
> diff --git a/server/reds.c b/server/reds.c
> index cdbb94cb..22198926 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -1149,7 +1149,7 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
>      }
>      spice_buffer_append(cmc, message, size);
>      if (sizeof(VDAgentMessage) > cmc->offset) {
> -        spice_debug("not enough data yet. %zd", cmc->offset);
> +        spice_debug("not enough data yet. %" G_GSSIZE_FORMAT, cmc->offset);
>          return;
>      }
>      msg_header = (VDAgentMessage *)cmc->buffer;
> @@ -1157,7 +1157,7 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
>          goto overflow;
>      }
>      if (msg_header->size > cmc->offset - sizeof(VDAgentMessage)) {
> -        spice_debug("not enough data yet. %zd", cmc->offset);
> +        spice_debug("not enough data yet. %" G_GSSIZE_FORMAT, cmc->offset);
>          return;
>      }
>      if (msg_header->size < sizeof(VDAgentMonitorsConfig)) {
> diff --git a/subprojects/spice-common b/subprojects/spice-common
> index 125cfa7f..6dc745e8 160000
> --- a/subprojects/spice-common
> +++ b/subprojects/spice-common
> @@ -1 +1 @@
> -Subproject commit 125cfa7fb2493060419c79eae3b40dbc499bd3db
> +Subproject commit 6dc745e877a12ed25634040dd6fa46fa5de694e2

Intentional?

Apart from this,

Reviewed-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>

Christophe

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]