[Spice-devel] [PATCH spice 7/9] red-channel: do not call pipe_add with NULL item

[Frediano Ziglio]

> 
> Hi
> 
> ----- Original Message -----
> > 
> > > 
> > > From: Marc-André Lureau <mlureau at redhat.com>
> > > 
> > > For some client, it might not be possible to create an item. In this
> > > case, don't call pipe_add() callback.
> > > 
> > 
> > Subject and comment assume that callback is always pipe_add which is
> > a wrong assumption.
> 
> I see three calls to red_channel_pipes_create_batch(), all with pipe_add
> callback (and that make sense, what else would you do with new pipe items?)
> 

This rationale is wrong from the design prospective.
If does not matter is there are one function that pass pipe_add as callback
or 30.000 functions all passing pipe_add as callback. The function is defined
as receiving a callback with a given signature. You could however specify with
additional documentation requirements and contract for the callback as for
instance that you will never pass a NULL item parameter.

> Anyway, I can just replace "pipe_add() callback" with "the callback" if you
> feel it's better.
> 
> > > Signed-off-by: Marc-André Lureau <mlureau at redhat.com>
> > > ---
> > >  server/red-channel.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/server/red-channel.c b/server/red-channel.c
> > > index ce4c691..fcbe868 100644
> > > --- a/server/red-channel.c
> > > +++ b/server/red-channel.c
> > > @@ -2242,10 +2242,12 @@ static void
> > > red_channel_pipes_create_batch(RedChannel
> > > *channel,
> > >      PipeItem *item;
> > >      int num = 0;
> > >  
> > > +    spice_assert(callback);
> > > +
> > >      RING_FOREACH_SAFE(link, next, &channel->clients) {
> > >          rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> > >          item = (*creator)(rcc, data, num++);
> > > -        if (callback) {
> > > +        if (item != NULL) {
> > >              (*callback)(rcc, item);
> > >          }
> > >      }

Frediano