>
> On Fri, Jun 03, 2016 at 12:00:24PM +0100, Frediano Ziglio wrote:
> > g_object_new is a variadic function which take property values.
> > As compiler cannot check if these property values are correct
> > make sure they are using casts.
> > This actully fix a crash in reds.c for 32 bit architectures.
> >
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > server/main-dispatcher.c | 2 +-
> > server/reds.c | 4 ++--
> > server/smartcard.c | 4 ++--
> > server/spicevmc.c | 4 ++--
> > 4 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/server/main-dispatcher.c b/server/main-dispatcher.c
> > index bc0de24..93de440 100644
> > --- a/server/main-dispatcher.c
> > +++ b/server/main-dispatcher.c
> > @@ -292,7 +292,7 @@ MainDispatcher* main_dispatcher_new(RedsState *reds,
> > SpiceCoreInterfaceInternal
> > MainDispatcher *self = g_object_new(TYPE_MAIN_DISPATCHER,
> > "spice-server", reds,
> > "core-interface", core,
> > - "max-message-type",
> > MAIN_DISPATCHER_NUM_MESSAGES,
> > + "max-message-type", (guint)
> > MAIN_DISPATCHER_NUM_MESSAGES,
> > NULL);
>
> The MAIN_DISPATCHER_NUM_MESSAGES message comes from an enum, so technically
> it could be as small as a 'char', but in practice it'll always be an int.
> In any case var-args will always promote types small that int, to be the
> size of an int.
>
> IOW, this cast shouldn't be needed, since it'll already be past as an
> int by the compiler.
>
Agreed, not needed. Just does not hurt as hint to check the type
if changes.
>
> > diff --git a/server/reds.c b/server/reds.c
> > index 4fd1d35..5ec73b7 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -4379,8 +4379,8 @@ static RedCharDeviceVDIPort
> > *red_char_device_vdi_port_new(RedsState *reds)
> > {
> > return g_object_new(RED_TYPE_CHAR_DEVICE_VDIPORT,
> > "spice-server", reds,
> > - "client-tokens-interval", REDS_TOKENS_TO_SEND,
> > - "self-tokens", REDS_NUM_INTERNAL_AGENT_MESSAGES,
> > + "client-tokens-interval", (guint64)
> > REDS_TOKENS_TO_SEND,
> > + "self-tokens", (guint64)
> > REDS_NUM_INTERNAL_AGENT_MESSAGES,
> > "opaque", reds,
> > NULL);
> > }
>
> This all make total sense though, since you're forcing use of a larger type
> than the compiler would otherwise use - it would use int by default,but you
> want a 64-bit type.
>
Yes, this is the bug. Variadic are promoted to int/unsigned int. In case of
64 bit platform usually is promoted to 64 bit as stated in the ABI so this
is not a problem there but for 32 bit (at least x86) the 64 bit will
be read from the 2 32 bit passes, the value and the next string pointer so
the REDS_NUM_INTERNAL_AGENT_MESSAGES will be interpreted as the next
property name!
>
> > diff --git a/server/smartcard.c b/server/smartcard.c
> > index 872aa1d..9b1b3d6 100644
> > --- a/server/smartcard.c
> > +++ b/server/smartcard.c
> > @@ -261,8 +261,8 @@ static RedCharDeviceSmartcard
> > *smartcard_device_new(RedsState *reds, SpiceCharDe
> > char_dev = g_object_new(RED_TYPE_CHAR_DEVICE_SMARTCARD,
> > "sin", sin,
> > "spice-server", reds,
> > - "client-tokens-interval", 0ULL,
> > - "self-tokens", ~0ULL,
> > + "client-tokens-interval", (guint64) 0ULL,
> > + "self-tokens", (guint64) ~0ULL,
> > NULL);
> >
> > g_object_set(char_dev, "opaque", char_dev, NULL);
> > diff --git a/server/spicevmc.c b/server/spicevmc.c
> > index b662d94..a863e39 100644
> > --- a/server/spicevmc.c
> > +++ b/server/spicevmc.c
> > @@ -580,8 +580,8 @@ red_char_device_spicevmc_new(SpiceCharDeviceInstance
> > *sin,
> > return g_object_new(RED_TYPE_CHAR_DEVICE_SPICEVMC,
> > "sin", sin,
> > "spice-server", reds,
> > - "client-tokens-interval", 0ULL,
> > - "self-tokens", ~0ULL,
> > + "client-tokens-interval", (guint64) 0ULL,
> > + "self-tokens", (guint64) ~0ULL,
> > "opaque", opaque,
> > NULL);
>
> AFAICT, these are all redundant since the ULL suffice on the constant
> should already ensure it is passed as a 64-bit type.
>
Not really portable assumption, in theory could be 128 bit or 256 in
a remote future. I think Gcc already support 128 bit integers.
> Regards,
> Daniel
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel