On Tue, 2016-12-20 at 05:38 -0500, Frediano Ziglio wrote: > > > > > > A series of patches refactoring the somewhat-confusing > > red_channel_client_init_send_data() function. The third argument to > > this > > function is a RedPipeItem and it was never very obvious when or why > > we should > > pass an item in this parameter. Sometimes callers passed NULL, and > > sometimes > > they passed an item. It turns out that it's only necessary to pass > > the item > > to > > this parameter if the pipe item needs to be kept alive until we > > guarantee > > that > > the message has been sent. We only need to do this when we have > > marshalled > > some > > data by reference, and that data is owned by the RedPipeItem. To > > make this > > more > > obvious, I've attempted to refactor things so that we no longer > > rely on this > > mechanism to keep the data alive, but instead reference the pipe > > item (or > > other > > structure that owns the referenced data) call _add_by_ref_full() to > > unreference > > the data after the message has been sent. > > > > Changes in v2: > > - removed one merged patch > > - this one compiles > > > > Changes in v3: > > - introduce a new patch (#1) that ensures that marshaller data is > > freed as > > soon as possible after the message is sent. This fixes an assert > > that > > Frediano > > ran into on the original patchset because some data was kept alive > > longer > > than > > expected. > > - Use Frediano's SET/INIT patch (4) instead of my original > > approach > > - squashed a fix (removing unused struct field) from Frediano into > > last > > patch. > > - removed another ACKed and pushed patch > > > > I would ack the entire series beside: > - I cannot ack my own patch; > - I'm not again moving marshaller_free_pipe_item into red-pipe- > item.c; > it's basically an utility for RedPipeItem. RedPipeItem, as the name > suggest are though to be put into the pipe and we know this pipe > it's the pipe that goes to the client so has to be marshaled. > Perhaps marshaller_unref_pipe_item instead of > marshaller_free_pipe_item > would also make sense (I noted it calls a unref and that you > added also a marshaller_unref_drawable). Yes, I forgot about this comment. I'll de-duplicate these functions and move it to red-pipe-item.h and sent another version _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel