Re: multiple capabilities in a single RPC

[Neal H. Walfield]

> Given such constraints, the actual solution is actually pretty simple,
> and I hacked up a patch that may do it.  Of course I was too lazy to
> even compile it.  But if you are lucky there are only typos and no
> semantical problems ;)

Thanks for working on this.

There were a couple of typos all easily fixed.

We need to move the declaration of struct _hurd_cap_list_item from
cap-server-internal.h to cap-server.h.  The whole point of the use
context is to create the list items on the stack and not allocate them
internally.


Your code gratuitously checks the inhibited state of the client.  We
know that the capability handle we are looking up belongs to the same
client as the first capability handle (i.e. the one that we are
dispatching the RPC on).  Hence, if we are running client->state must
be either green or yellow.  You assert this yourself at the start of
the function:

  /* Check if both capability handles belong to the same client.  */
  if (_hurd_cap_client_id (cap) != _hurd_cap_client_id (ctx->cap))
    return EINVAL;


Is it worth adding the obj pointer to the use context?  Internally, we
can trivially find it given the _obj_entry?  Then we can mark the
whole structure as private.  Of course, we would have to return the
capability object in one of the arguments but I don't think this is
bad.


SAME_CLASS argument is really useless.  All it does is offer another
assert.  If the argument is that it provides type checking (and we
need a mechanism to do that), I think a more useful type check would
be to have the user pass a pointer to a capability object class
indicating what class the capability must be in.  This is a clean way
to provide type checking.  The alternative is to add an accessor
function for capability objects which returns the capability object's
capability class (or allowing the user to inspect the structure
directly).  If we incorporate type checking into the start_cap_use
function directly, this makes the case where a stub accepts multiple
types difficult as it must call _start_cap_use multiple times, once
for each type it accepts, until it succeeds.  We may just want to punt
this until we see a real need but adding an accessor function to
return a capability object's class is reasonably elegant.


If we require the user to pass the rpc context to _end_cap_use then we
can elide the _status member.  All values are easily determined
anyways.


Finally, I think the cap field name should be changed to handle.  In
both the cap use context and the rpc context.  This is confusing.  Why
is a capability object not a cap but an obj?  I think of a capability
as the whole she-bang and I think we should call capability handles
handles to avoid any confusion in that regard.

> There are probably some error values which should be changed, because
> they are more suitable for the demuxer internals than for an actual
> user.  I am thinking about the ECAP_* error codes.

Yes, I think so.  ECAP_* are not actually exported and I don't think
we want to export them.  Maybe ESRCH if the cap is not found and EIEIO
if it died.

> You can not lock two objects at the same time, unless you define a
> locking hierarchy for all the objects in the set, or find some other
> dead-lock-avoidance strategy.

In case it wasn't obvious, there is a trivial general solution: we
lexically sort the capability objects by their location and use that
as the locking order.

Thanks,
Neal