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

[Uri Lublin]

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.

Uri.