[Spice-devel] [PATCH] fixup! Change Drawable->pipes from Ring to GList

[Jonathon Jongsma]

On Fri, 2016-09-16 at 12:25 -0400, Frediano Ziglio wrote:
> > 
> > 
> > Fix potential infinite loop
> > ---
> > You can still introduce a foreach macro if you'd like, but this
> > should fix
> > the
> > infinite loop for now.
> > 
> 
> Macro could be introduced back later if we decide to.
> 
> > 
> >  server/stream.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/server/stream.c b/server/stream.c
> > index 934b236..be3e437 100644
> > --- a/server/stream.c
> > +++ b/server/stream.c
> > @@ -331,7 +331,6 @@ void detach_stream(DisplayChannel *display,
> > Stream
> > *stream)
> >  static void before_reattach_stream(DisplayChannel *display,
> >                                     Stream *stream, Drawable
> > *new_frame)
> >  {
> > -    RedDrawablePipeItem *dpi;
> >      DisplayChannelClient *dcc;
> >      int index;
> >      StreamAgent *agent;
> > @@ -352,8 +351,8 @@ static void
> > before_reattach_stream(DisplayChannel
> > *display,
> >      index = display_channel_get_stream_id(display, stream);
> >      dpi_item = stream->current->pipes;
> >      while (dpi_item) {
> > -        GList *dpi_next = dpi_item->next;
> > -        dpi = dpi_item->data;
> > +        RedDrawablePipeItem *dpi = dpi_item->data;
> > +        dpi_item = dpi_item->next;
> >          dcc = dpi->dcc;
> >          agent = dcc_get_stream_agent(dcc, index);
> >  
> 
> Wouldn't a
> 
>    for (; dpi_item; dpi_item = dpi_next) 
> 
> be enough (moving dpi_next declaration outside) ?
> 
> Having dpi_item as the next item could be confusing as it's used
> for the current item in other contexts.

> On the same naming dpi_item is not a dpi (RedDrawablePipeItem) which
> could be confusing too. A dpi_link perhaps would be better or reuse
> directly the link/link_next variable.

Sure, that could work as well. I just used this approach because it was
a smaller change. I did have the same concerns you mentioned and
dismissed them at the time. But I'll make the change.



> 
> > 
> > @@ -373,7 +372,6 @@ static void
> > before_reattach_stream(DisplayChannel
> > *display,
> >                  ++agent->drops;
> >              }
> >          }
> > -        dpi_item = dpi_next;
> >      }
> >  
> 
> Frediano