> > On 12/21/2015 12:33 PM, 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. > > > Hi Frediano, > > This patchset does associate between the internal core and the worker > (only for display channels). > However context was already being passed to core function via the > opaque parameter. The internal context does not show up when the > callback is called (as that would change the API), only "opaque" > is available. > > Thanks, > Uri. > The association between internal core and worker is done by worker on the internal core built by worker and is used by cursor and display channels. In SpiceTimer *(*timer_add)(SpiceTimerFunc func, void *opaque); (or similar callbacks) opaque is the context for func but I want the context for timer_add callback which is not available. I used in my patches SpiceCoreInterfaceInternal* for the context. This change the internal API but not the public one so it's not an issue. I don't understand where does your misinterpretation came. Are you suggesting a different comment or a different implementation? Perhaps are you suggesting to use another void* pointer for function context instead of SpiceCoreInterfaceInternal*? I can do it, I don't know how big would be the change. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel