[Spice-devel] [PATCH spice-common] protocol: Use a typedef to specify stream_id type

[Christophe de Dinechin]

> On 10 May 2018, at 18:08, Jonathon Jongsma <jjongsma at redhat.com> wrote:
> 
> On Sat, 2018-05-05 at 16:43 +0100, Frediano Ziglio wrote:
>> This change does not affect generated code but make source more
>> readable. Also document in a single location the range of this
>> type.
>> 
>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>> ---
>> There are no other typedef beside fixed28_4, so the "_t" suffix is
>> used only here. Maybe would be better to use the name "StreamId"?
>> Or the "_type" suffix used by some enumeration type?
> 
> I'm not sure that it personally makes a big difference in 'readability'
> for me, but I'm fine with the change. I prefer the using the _t suffix
> for integer types like this.

Just curious if you wanted to say you prefer using a suffix or not? (Confused by “the using”, wonder if it’s a typo)

I personally do like this change.

> 
> 
> Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
> 
> 
> 
>> ---
>> spice.proto | 17 +++++++++++------
>> 1 file changed, 11 insertions(+), 6 deletions(-)
>> 
>> diff --git a/spice.proto b/spice.proto
>> index 6f873a2..4d916bb 100644
>> --- a/spice.proto
>> +++ b/spice.proto
>> @@ -4,6 +4,11 @@
>> 
>> typedef fixed28_4 int32 @ctype(SPICE_FIXED28_4);
>> 
>> +/* IDs of the video stream messages.
>> + * These IDs are in the interval [0, SPICE_MAX_NUM_STREAMS)
>> + */
>> +typedef stream_id_t uint32;
>> +
>> struct Point {
>>     int32 x;
>>     int32 y;
>> @@ -698,7 +703,7 @@ struct String {
>> };
>> 
>> struct StreamDataHeader {
>> -	uint32 id;
>> +	stream_id_t id;
>> 	uint32 multi_media_time;
>> };
>> 
>> @@ -756,7 +761,7 @@ channel DisplayChannel : BaseChannel {
>> 
>>     message {
>> 	uint32 surface_id;
>> -	uint32 id;
>> +	stream_id_t id;
>> 	stream_flags flags;
>> 	video_codec_type codec_type;
>> 	uint64 stamp;
>> @@ -775,12 +780,12 @@ channel DisplayChannel : BaseChannel {
>>     } stream_data;
>> 
>>     message {
>> -	uint32 id;
>> +	stream_id_t id;
>> 	Clip clip;
>>     } stream_clip;
>> 
>>     message {
>> -	uint32 id;
>> +	stream_id_t id;
>>     } stream_destroy;
>> 
>>     Empty stream_destroy_all;
>> @@ -954,7 +959,7 @@ channel DisplayChannel : BaseChannel {
>>     } draw_composite;
>> 
>>     message {
>> -        uint32 stream_id;
>> +        stream_id_t stream_id;
>>         uint32 unique_id;
>>         uint32 max_window_size;
>>         uint32 timeout_ms;
>> @@ -986,7 +991,7 @@ channel DisplayChannel : BaseChannel {
>>     } init = 101;
>> 
>>     message {
>> -        uint32 stream_id;
>> +        stream_id_t stream_id;
>>         uint32 unique_id;
>>         // the mm_time of the first frame included in the report
>>         uint32 start_frame_mm_time;
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel