Hey, On Mon, Dec 21, 2015 at 10:33:08AM +0000, Frediano Ziglio wrote: > This patchset want to solve the problem of not having a context in > SpiceCoreInterface. SpiceCoreInterface defines a set of callbacks to > handle events in spice-server. These callbacks allow to handle timers, > watch for file descriptors and send channel events. All these callbacks > does not accept a context (usually in C passed as a void* parameter) so > they is hard for them to differentiate the interface specified. > Unfortunately this structure is used even internally from different > contextes for instance every RedWorker thread have a different context. To > solve this issue some workarounds are used. Currently for timers a variable > depending on the current thread is used while for watches the opaque > parameter to pass to the event callback is used as it currently points just > to RedChannelClient structure. This however impose some implicit > maintanance problem in the future. What happen for instance if for some > reason a timer is registered during worker initialization, run in another > thread? What if we decide to register a file descriptor callback for > something not a RedChannelClient? Could be that the program will run > without any issue till some bytes changes and weird thing could happen. > > The implementation of this solution is done implementing an internal "core" > interface that have context specification and use it to differentiate the > context instead of relying to some other, hard to maintain, detail. Then an > adapter structure (name inpired to the adapter pattern) will provide the > internal core interface using the external, public, definition (in the > future this technique can be used to extend the external interface without > breaking the ABI). > > First patches are intended to be just mechanical changes to make easier to > understand the idea. The idea if this solution is accepted is to squash > them all in a single patch. For this reason patches comments are quite > small and will be replaced by top of this one. Note that not all callbacks > from the public SpiceCoreInterface have a context (a "const > SpiceCoreInterfaceInternal *" argument) but this can be easily extended. > Last patch is the main reason for these patches. The approach looks good to me, thanks for spending time on that. I'd keep the patches separate though as it's easier to see what's going on than if it's all done in a single big patch. Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel