[PATCH] randr: stop clients from deleting immutable output properties

Luc Verhaegen libv at skynet.be
Wed Aug 10 08:45:27 PDT 2011


On Wed, Aug 10, 2011 at 08:38:01AM -0700, Keith Packard wrote:
> On Wed, 10 Aug 2011 15:48:30 +0200, Luc Verhaegen <libv at skynet.be> wrote:
> 
> > This was not done for a few reasons:
> > 1) it clearly splits between clientside and server internal usage. The 
> > comment that i added to RRDeleteOutputProperty in the very first version
> > of the patch underlined that this was only for server internal use due
> > to immutable (this was the code tested on the N9, but was never 
> > committed).
> > 2) ValidAtom is only ever called in the client side functions, while
> > the check for immutable is once in general and once in client side,
> > putting the balance client side, so i picked client side.
> > 3) it is much more invasive.
> 
> I'd prefer to see these two approaches mixed.
> 
> I like not duplicating code, which means using RRDeleteOutputProperty
> and having that take the 'force' parameter. It's a bit ugly in that
> server internal paths will end up casting the return to (void) as the
> function cannot fail when force == TRUE, but I balance that with not
> duplicating the whole function with one check inserted.
> 
> But, I also think the ValidAtom check should only be in the client path;
> I would have to go look at the server reset code carefully to know
> whether the atom table is cleared before or after the RandR outputs are
> destroyed. Not changing the semantics of RRDeleteOutputProperty as it is
> used internally seems fairly important.

A cleaner way, which keeps us from nowing the internal structure of 
the property list, and which does not require us to change any 
functions besides the ProcRRDeleteOutputProperty, is:
* call ValidAtom(stuff->property) [throw BadAtom]
* retrieve the property structure by calling 
RRQueryOutputProperty(output, stuff->property) [throw BadName]
* check the property's immutable flag [throw BadAccess]
* call RRDeleteOutputProperty(output, stuff->property)
* return Success

But all the other client side handlers walk the list manually.

Luc Verhaegen.



More information about the xorg-devel mailing list