On Tue, May 03, 2011 at 01:52:38AM +0200, Marc-André Lureau wrote: > On Tue, Apr 26, 2011 at 12:54 PM, Alon Levy <[email protected]> wrote: > > Introduce functions to add (via producer method) the same item to multiple > > pipes, all for the same channel. > > > > Note: Right now there is only a single channel, but the next patches will > > do the > > per-channel breakdown to channel and channel_client before actually > > introducing > > a ring in RedChannel, this makes it easier to make smaller changes - the > > channel->rcc link will exist until removed in the ring introducing patch. > > --- > > server/red_channel.c | 51 > > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > server/red_channel.h | 12 ++++++++++- > > 2 files changed, 62 insertions(+), 1 deletions(-) > > > > diff --git a/server/red_channel.c b/server/red_channel.c > > index aff99ed..7e1edbe 100644 > > --- a/server/red_channel.c > > +++ b/server/red_channel.c > > @@ -1051,4 +1051,55 @@ void red_client_set_main(RedClient *client, > > MainChannelClient *mcc) { > > client->mcc = mcc; > > } > > > > +/* > > + * Functions to push the same item to multiple pipes. > > + */ > > + > > +/* > > + * TODO: after convinced of correctness, add paths for single client > > + * that avoid the whole loop. perhaps even have a function pointer table > > + * later. > > + * TODO - inline? macro? right now this is the simplest from code amount > > + */ > > + > > +typedef void (*rcc_item_t)(RedChannelClient *rcc, PipeItem *item); > > +typedef int (*rcc_item_cond_t)(RedChannelClient *rcc, PipeItem *item); > > > > +void __red_channel_pipes_create_batch(RedChannel *channel, > > + new_pipe_item_t creator, void *data, > > + rcc_item_t visitor) > > The "__" prefix is not necessary imho, however a static modifier would be > nice. > > As you know already, I am not fond of the "visitor" name, since here > it's just a callback, and not a real visitor "object" (ie with a state > and multiple visit() methods etc..). > > > +{ > > + RedChannelClient *rcc; > > + PipeItem *item; > > + int num = 0; > > + > > + if (!(rcc = channel->rcc)) { > > + return; > > + } > > + item = (*creator)(rcc, data, num++); > > + if (visitor) { > > + (*visitor)(rcc, item); > > + } > > +} > > + > > +void red_channel_pipes_new_add_push(RedChannel *channel, > > + new_pipe_item_t creator, void *data) > > +{ > > + __red_channel_pipes_create_batch(channel, creator, data, > > + red_channel_client_pipe_add); > > + red_channel_push(channel); > > +} > > + > > +void red_channel_pipes_new_add(RedChannel *channel, new_pipe_item_t > > creator, void *data) > > +{ > > + __red_channel_pipes_create_batch(channel, creator, data, > > + red_channel_client_pipe_add); > > +} > > + > > +// despite the name, this should push (TODO: match function to name) > > yeah... that could be fixed easily instead of left as a TODO. No? > > Imho, the responsibility to call push() should be left to the caller > instead... > > > +void red_channel_pipes_new_add_tail(RedChannel *channel, new_pipe_item_t > > creator, void *data) > > +{ > > + __red_channel_pipes_create_batch(channel, creator, data, > > + > > red_channel_client_pipe_add_tail_no_push); > > + red_channel_push(channel); > > +} > > > diff --git a/server/red_channel.h b/server/red_channel.h > > index 877c567..15d70b6 100644 > > --- a/server/red_channel.h > > +++ b/server/red_channel.h > > @@ -271,6 +271,16 @@ void > > red_channel_client_set_message_serial(RedChannelClient *channel, uint64_t); > > void red_channel_client_begin_send_message(RedChannelClient *rcc); > > > > void red_channel_pipe_item_init(RedChannel *channel, PipeItem *item, int > > type); > > + > > +// TODO: add back the channel_pipe_add functionality - by adding reference > > counting > > +// to the PipeItem. > > + > > +// helper to push a new item to all channels > > +typedef PipeItem *(*new_pipe_item_t)(RedChannelClient *rcc, void *data, > > int num); > > What is "num" doing? >
This is meant to be used by the new function when adding to multiple pipes. The num goes from 0 to n-1 when adding to n pipes (for a channel with n clients). > Would it be more future proof to take PipeItem * as argument, that can > be either referenced or copied by the various pipes? I guess that's possible, so it would be NULL for the first and non NULL for the rest? sounds like it would work for the only use case I had for num (where I tested num!=0). > (see general comment in 00/62 reply about sharing PipeItem for various pipes) > See my response to same. > > +void red_channel_pipes_new_add_push(RedChannel *channel, new_pipe_item_t > > creator, void *data); > > +void red_channel_pipes_new_add(RedChannel *channel, new_pipe_item_t > > creator, void *data); > > +void red_channel_pipes_new_add_tail(RedChannel *channel, new_pipe_item_t > > creator, void *data); > > + > > void red_channel_client_pipe_add_push(RedChannelClient *rcc, PipeItem > > *item); > > void red_channel_client_pipe_add(RedChannelClient *rcc, PipeItem *item); > > void red_channel_client_pipe_add_after(RedChannelClient *rcc, PipeItem > > *item, PipeItem *pos); > > @@ -374,4 +384,4 @@ void red_client_destroy(RedClient *client); > > void red_client_set_main(RedClient *client, MainChannelClient *mcc); > > MainChannelClient *red_client_get_main(RedClient *client); > > > > -#endif > > +#endif > > \ No newline at end of file > > -- > > 1.7.4.4 > > > > _______________________________________________ > > Spice-devel mailing list > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/spice-devel > > > > > > -- > Marc-André Lureau _______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
