[Spice-devel] [spice] server: Make sure g_object_new receive the correct data

[Christophe Fergeau]

On Tue, Jul 26, 2016 at 02:35:53PM +0300, Uri Lublin wrote:
> On 07/25/2016 07:51 PM, Francois Gouget wrote:
> > On Mon, 25 Jul 2016, Frediano Ziglio wrote:
> > [...]
> > > > > -                        "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,
> > [...]
> > > > The patch is ok, but I think it would be better
> > > > to do the cast in the define itself, or replace
> > > > the define with a const (g)uint64_t variable
> > > > 
> > > > Uri.
> > > 
> > > This would bound the constant to the property which does
> > > not make much sense as the constant can be used for
> > > different purposes. What if the same constant is used for
> > > two properties with different types?
> 
> I do not see it as a problem. Constants have meaning.
> What if the constant is used in 10 places (as uint64), you
> will need to cast all places.
> (Theoretically, not in this case as Francois mentions below)
> 
> > 
> > To be fair REDS_TOKENS_TO_SEND is only used in this one place. But
> > REDS_NUM_INTERNAL_AGENT_MESSAGES is defined in main-channel.h and is
> > used to define MAIN_CHANNEL_RECEIVE_BUF_SIZE so defining it as a guint64
> > may not make sense.
> > 
> > Also I think having a visible cast here more explicitly indicates that
> > the property is 64 bit than a cast hidden in a far away macro.
> > 
> > (One could also argue for an explicit comment but I think that would be
> > overkill. Why add a comment here and not for every other cast?)
> > 
> 
> I do not see the value of being explicit about this property being
> a 64 bit. I agree that a comment would be an overkill.
> 
> Having said that, I accept the patch as is.

Ok, I've pushed this patch then, thanks everyone!

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160727/5b972b60/attachment.sig>