Re: Alexandre Julliard : server: Added access rights mapping to synchronization objects.

[Robert Shearman]

Alexandre Julliard wrote:
> @@ -132,6 +133,15 @@ static int event_satisfied( struct objec
>     return 0;  /* Not abandoned */
> }
>
> +static unsigned int event_map_access( struct object *obj, unsigned int access )
> +{
> - if (access & GENERIC_READ)    access |= STANDARD_RIGHTS_READ | SYNCHRONIZE | EVENT_QUERY_STATE;
> - if (access & GENERIC_WRITE)   access |= STANDARD_RIGHTS_WRITE;
> - if (access & GENERIC_EXECUTE) access |= STANDARD_RIGHTS_EXECUTE;
> - if (access & GENERIC_ALL)     access |= STANDARD_RIGHTS_ALL | EVENT_ALL_ACCESS;
> - return access & ~(GENERIC_READ | GENERIC_WRITE | GENERIC_EXECUTE | GENERIC_ALL);
>
> +}
> -
>
> static int event_signal( struct object *obj, unsigned int access )
> {
>     struct event *event = (struct event *)obj;

Why did you decide to rely on another function to map the access rights? 
It seems that none of the objects' mappings depend on any state in the 
object, so couldn't you have just put the access rights that 
GENERIC_READ, GENERIC_WRITE, GENERIC_EXECUTE and GENERIC_ALL map to 
somewhere associated with the object and have just one mapping function.

Alexandre Julliard : server: Added access rights mapping to synchronization objects.

[Alexandre Julliard]

Robert Shearman  writes:
> Why did you decide to rely on another function to map the access rights? 
> It seems that none of the objects' mappings depend on any state in the 
> object, so couldn't you have just put the access rights that 
> GENERIC_READ, GENERIC_WRITE, GENERIC_EXECUTE and GENERIC_ALL map to 
> somewhere associated with the object and have just one mapping function.

I think it's cleaner to use functions in the ops structure rather than
starting to mix various data pointers in there too.

Alexandre Julliard : server: Added access rights mapping to synchronization objects.

[Robert Shearman]

Alexandre Julliard wrote:
> Robert Shearman  writes:
> > Why did you decide to rely on another function to map the access rights? 
> > It seems that none of the objects' mappings depend on any state in the 
> > object, so couldn't you have just put the access rights that 
> > GENERIC_READ, GENERIC_WRITE, GENERIC_EXECUTE and GENERIC_ALL map to 
> > somewhere associated with the object and have just one mapping function.
>
> I think it's cleaner to use functions in the ops structure rather than
> starting to mix various data pointers in there too.

How about adding a new structure that contains the ops structure and the 
data then to not mix them then?

Alexandre Julliard : server: Added access rights mapping to synchronization objects.

[Alexandre Julliard]

Robert Shearman  writes:
> How about adding a new structure that contains the ops structure and the 
> data then to not mix them then?

I don't see any reason to make the data structure more complex. The
amount of code is really pretty much the same, putting it in a table
won't make much difference. Also a function is a lot more flexible,
even if the results don't depend on the specific object we may at some
point want to make them depend on something else, and using a constant
table would needlessly limit our options.