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.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel