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

[Jonathon Jongsma]

On Fri, 2018-05-11 at 17:56 +0200, Christophe de Dinechin wrote:
> > 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.
> 


Oops. I meant to say that I like stream_id_t better than the other
options Frediano mentioned.

Jonathon

> > 
> > 
> > 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
> 
>