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

[Luc Verhaegen]

On Wed, Aug 10, 2011 at 01:44:57PM +0100, Daniel Stone wrote:
> On Wed, Aug 10, 2011 at 01:06:33PM +0200, Luc Verhaegen wrote:
> > Immutable in randr means that clients are not able to alter the
> > property itself, they are only allowed to alter the property value.
> > This logically means that the property then should not be deleted
> > by the client either.
> 
> Instead of inlining RRDeleteOutputProperty here, you could do something
> like this.
> 
> Cheers,
> Daniel
> 
> diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
> index c2814d4..481cabb 100644
> --- a/hw/xfree86/modes/xf86Crtc.c
> +++ b/hw/xfree86/modes/xf86Crtc.c
> @@ -2920,7 +2920,7 @@ xf86OutputSetEDIDProperty (xf86OutputPtr output, void *data, int data_len)
>  	RRChangeOutputProperty(output->randr_output, edid_atom, XA_INTEGER, 8,
>  			       PropModeReplace, data_len, data, FALSE, TRUE);
>      } else {
> -	RRDeleteOutputProperty(output->randr_output, edid_atom);
> +	RRDeleteOutputProperty(output->randr_output, edid_atom, TRUE);
>      }
>  }
>  
> diff --git a/randr/randrstr.h b/randr/randrstr.h
> index 80aed68..504aacf 100644
> --- a/randr/randrstr.h
> +++ b/randr/randrstr.h
> @@ -860,8 +860,8 @@ RRGetOutputProperty (RROutputPtr output, Atom property, Bool pending);
>  extern RRPropertyPtr
>  RRQueryOutputProperty (RROutputPtr output, Atom property);
>  		       
> -extern void
> -RRDeleteOutputProperty (RROutputPtr output, Atom property);
> +extern int
> +RRDeleteOutputProperty (RROutputPtr output, Atom property, Bool force);
>  
>  extern Bool
>  RRPostPendingProperties (RROutputPtr output);
> diff --git a/randr/rrproperty.c b/randr/rrproperty.c
> index ba04c16..0805d28 100644
> --- a/randr/rrproperty.c
> +++ b/randr/rrproperty.c
> @@ -118,16 +118,27 @@ RRCreateOutputProperty (Atom property)
>      return prop;
>  }
>  
> -void
> -RRDeleteOutputProperty(RROutputPtr output, Atom property)
> +/**
> + * Deletes a property on the specified output.
> + *
> + * @param force If TRUE, will also delete immutable properties.
> + * @ret Success, or an X error code.
> + */
> +int
> +RRDeleteOutputProperty(RROutputPtr output, Atom property, Bool force)
>  {
>      RRPropertyRec *prop, **prev;
>  
> +    if (!ValidAtom(property))
> +        return BadAtom;
> +
>      for (prev = &output->properties; (prop = *prev); prev = &(prop->next))
>  	if (prop->propertyName == property) {
>              *prev = prop->next;
> +            if (prop->immutable && !force)
> +                return BadAccess;
>              RRDeleteProperty(output, prop);
> -            return;
> +            return Success;
>          }
>  }
>  
> @@ -552,20 +563,16 @@ ProcRRDeleteOutputProperty (ClientPtr client)
>  {
>      REQUEST(xRRDeleteOutputPropertyReq);
>      RROutputPtr	output;
> -              
> +    int ret;
> +
>      REQUEST_SIZE_MATCH(xRRDeleteOutputPropertyReq);
>      UpdateCurrentTime();
>      VERIFY_RR_OUTPUT(stuff->output, output, DixReadAccess);
> -    
> -    if (!ValidAtom(stuff->property))
> -    {
> -	client->errorValue = stuff->property;
> -	return BadAtom;
> -    }
>  
> -
> -    RRDeleteOutputProperty(output, stuff->property);
> -    return Success;
> +    ret = RRDeleteOutputProperty(output, stuff->property, FALSE);
> +    if (ret != Success)
> +        client->errorValue = stuff->property;
> +    return ret;
>  }
>  
>  int

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.

Luc Verhaegen.