On 29.07.2016 01:54, Keith Packard wrote: > Michel Dänzer <[email protected]> writes: > >> Without this change, there's no way for a client to explicitly change >> or destroy an existing event mask entry. Trying to do so as specified >> above would actually result in a new entry being created with >> the mask value passed in. > > Yup, this looks like a good thing to fix. > > Instead of walking the linked list, you should perform a resource lookup > instead using the provided ID; that would be more consistent, and would > let you catch errors where the provided resource ID was actually being > used on a different window. If it does, you should return a Match error. > > You've also lost the call to LEGAL_NEW_RESOURCE in the path which > creates a new event. > >> While we're at it, fix a memory leak if AddResource fails. > > You've made a very common mistake -- AddResource actually calls the > delete function when it fails, so you don't need to free the object in > the failure path. > > Here's an alternate (untested) patch which shows what I meant; use > whatever bits of this seem useful.
Thanks, I incorporated almost all your changes in v2, with the exception of not calling LEGAL_NEW_RESOURCE if mask == 0, due to this spec language: If eventContext is an unused XID, then if eventMask is empty no operation is performed. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
signature.asc
Description: OpenPGP digital signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
