[Spice-devel] [PATCH spice-server 1/2] Fix compile warnings on Linux 32bit system
Uri Lublin
uril at redhat.com
Thu Jun 13 07:56:15 UTC 2019
On 6/12/19 9:30 AM, Frediano Ziglio wrote:
> Some typos/improves (just commit message)
>
>> Subject: [PATCH spice-server 1/2] Fix compile warnings on Linux 32bit system
>>
>
> Better "Remove compile warnings ..."
Both are OK.
>
>> Based on a patch from Hongzhi.Song <hongzhi.song at windriver.com>.
>>
>> There are following compile errors on Linux 32bit system with -Werror
>> for gcc.
>>
>> red-channel.c:207:73: error: format '%x' expects argument of type
>> 'unsigned int', but argument 7 has type 'long unsigned int' [-Werror=format=]
>> |207| red_channel_debug(self, "thread_id 0x%" G_GSIZE_MODIFIER "x",
>> ~~~~~~~~~~~~~~~~~~~~~^
>> self->priv->thread_id);
>> ~~~~~~~~~~~~~~~~~~~~~^
>>
>> pthread_t is an opaque type so there it's not easy to get portably
s/portably/<something>/
maybe "... so there is no easy way to make the printf
format string portable"
>
> Remove "there"
>
>> the printf format string. However the type must be comparable in C
>> so this (excluding floating point which does not make sense) means
>> an integral type or a pointer.
>> Under *BSD this is a pointer so can be converted without loosing
>> precision to void*.
>> Under Linux this is a "unsigned long int" type, being Linux LP32 or
>
> It's ILP32, not LP32.
Since it's "unsigned long int" it's the same (IIUC).
>
>> LP64 this means that the size of pthread_t is the same as size_t >> so can be converted without loosing precision to void*.
>> Under MingW (the pthread port to Windows) this is a uintptr_t type
>> that is can be converted without loosing precision to void*.
>> On any potential future platforms if the integral type is smaller
>> than a uintptr_t type (which has the same size of void*) the cast
>> should trigger a warning and if not won't loose precision; the
>> integral type is unlikely to be bigger than a pointer and likely
>> the cast would trigger a warning.
>>
>> The cast on read_binary (red-replay-qxl.c) is safe, "*size" is a
>> size_t while "strm.total_out" is the number of written bytes in
>> a buffer which cannot be bigger than a size_t.
>>
>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Ack.
Uri.
>> ---
>> server/red-channel.c | 8 ++++----
>> server/red-client.c | 14 +++++++-------
>> server/red-replay-qxl.c | 2 +-
>> 3 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/server/red-channel.c b/server/red-channel.c
>> index 82e522395..e09edacf8 100644
>> --- a/server/red-channel.c
>> +++ b/server/red-channel.c
>> @@ -202,7 +202,7 @@ red_channel_constructed(GObject *object)
>> {
>> RedChannel *self = RED_CHANNEL(object);
>>
>> - red_channel_debug(self, "thread_id 0x%" G_GSIZE_MODIFIER "x",
>> self->priv->thread_id);
>> + red_channel_debug(self, "thread_id %p", (void*) self->priv->thread_id);
>>
>> RedChannelClass *klass = RED_CHANNEL_GET_CLASS(self);
>>
>> @@ -473,11 +473,11 @@ void red_channel_remove_client(RedChannel *channel,
>> RedChannelClient *rcc)
>>
>> if (!pthread_equal(pthread_self(), channel->priv->thread_id)) {
>> red_channel_warning(channel,
>> - "channel->thread_id (0x%" G_GSIZE_MODIFIER "x)
>> != "
>> - "pthread_self (0x%" G_GSIZE_MODIFIER "x)."
>> + "channel->thread_id (%p) != "
>> + "pthread_self (%p)."
>> "If one of the threads is != io-thread && !=
>> vcpu-thread, "
>> "this might be a BUG",
>> - channel->priv->thread_id, pthread_self());
>> + (void*) channel->priv->thread_id, (void*)
>> pthread_self());
>> }
>> spice_return_if_fail(channel);
>> link = g_list_find(channel->priv->clients, rcc);
>> diff --git a/server/red-client.c b/server/red-client.c
>> index 961b4970e..a4c79a174 100644
>> --- a/server/red-client.c
>> +++ b/server/red-client.c
>> @@ -174,11 +174,11 @@ void red_client_migrate(RedClient *client)
>> RedChannel *channel;
>>
>> if (!pthread_equal(pthread_self(), client->thread_id)) {
>> - spice_warning("client->thread_id (0x%" G_GSIZE_MODIFIER "x) != "
>> - "pthread_self (0x%" G_GSIZE_MODIFIER "x)."
>> + spice_warning("client->thread_id (%p) != "
>> + "pthread_self (%p)."
>> "If one of the threads is != io-thread && !=
>> vcpu-thread,"
>> " this might be a BUG",
>> - client->thread_id, pthread_self());
>> + (void*) client->thread_id, (void*) pthread_self());
>> }
>> FOREACH_CHANNEL_CLIENT(client, rcc) {
>> if (red_channel_client_is_connected(rcc)) {
>> @@ -193,12 +193,12 @@ void red_client_destroy(RedClient *client)
>> RedChannelClient *rcc;
>>
>> if (!pthread_equal(pthread_self(), client->thread_id)) {
>> - spice_warning("client->thread_id (0x%" G_GSIZE_MODIFIER "x) != "
>> - "pthread_self (0x%" G_GSIZE_MODIFIER "x)."
>> + spice_warning("client->thread_id (%p) != "
>> + "pthread_self (%p)."
>> "If one of the threads is != io-thread && !=
>> vcpu-thread,"
>> " this might be a BUG",
>> - client->thread_id,
>> - pthread_self());
>> + (void*) client->thread_id,
>> + (void*) pthread_self());
>> }
>> red_client_set_disconnecting(client);
>> FOREACH_CHANNEL_CLIENT(client, rcc) {
>> diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
>> index 6d3481805..fa44fa7c4 100644
>> --- a/server/red-replay-qxl.c
>> +++ b/server/red-replay-qxl.c
>> @@ -265,7 +265,7 @@ static replay_t read_binary(SpiceReplay *replay, const
>> char *prefix, size_t *siz
>> }
>> if ((ret = inflate(&strm, Z_NO_FLUSH)) != Z_STREAM_END) {
>> spice_error("inflate error %d (disc: %" G_GSSIZE_FORMAT ")",
>> - ret, *size - strm.total_out);
>> + ret, (size_t) (*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
>
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
More information about the Spice-devel
mailing list