Unnecessary default-constructibility requirements for RPC types?

[Norman Feske]

Hi Edgar,

thanks for posting this impressive (and unexpected) line of work! As far
as I know, this is the first time that someone other than me touched the
RPC template code of Genode. ;-)

I especially like the changes of the return-value handling. I was never
quite satisfied with this part. Your implementation is much nicer.

> Software patterns like RAII [3] are a possible reason for removing  
> that limitation. Quite often classes cannot provide a useful  
> default-constructor which does more than merely setting the object  
> state to "invalid". Because often there are no useful default values.  
> So if a class provides a default-constructor which just sets the  
> object to "invalid", the code complexity increases since care has to  
> be taken that the object is fed with data up to a specific point of  
> time.

I agree.

> Unfortunately, my implementation does not remove default-constructor  
> calls completely:
> 1. Capabilities still need default-constructors. Msgbuf stores them  
> directly in an array and I did not find the time to check changes of  
> this storage behavior for possible side effects.

This is not a problem because a default-constructed 'Capability' has a
well-defined 'invalid' state, which is expected (like for the
boost::optional types you cited).

> 2. Output-only _arguments_ have to be default-constructible too.  
> Please note, that non-default-constructible _return_ types are  
> allowed. So this affects only RPC arguments which are explicitly  
> declared as Output-only, for example by adding custom specializations  
> of Rpc_direction. The reason for this limitation is that my Pod_tuple  
> version still stores the RPC arguments directly. While input arguments  
> are copy-constructed, it is hard to copy from an object that does not  
> yet exist.

I doubt that this is a problem in practice. We are not using custom
specializations of 'Rpc_direction' at all. If one needs an output-only
argument, it should be transferred as return value, which can be a POD
struct to carry multiple values. The only use case where an output-only
argument would make sense is the delegation of a capability (which
cannot by carried in a POD struct) in addition to a return value. But as
'Capability' has a default constructor, this is not an issue.

> I successfully tested the code with the repositories "hello_tutorial"  
> and "demo" on base-linux. But if you decide to use my code  
> contribution, please note that it is rather a proof of concept. There  
> is room for improvements:
> * I tried to minimize the code modifications. So some refactoring may  
> be useful:

Thanks. That is good for the review.

> I expanded Meta::Empty by constructors and a typedef,

I haven't yet wrapped my head around this change yet. Give me some time
to let it sink in.

> I inserted possibly confusing forward declarations, and Meta::Pod_tuples
> and Meta::Pod_args could be renamed because they do not only accept  
> POD types [6].

I agree. With your changes, the (renamed) template should be moved
outside of 'meta.h', preferably to 'base/rpc.h'.

> * I completely ignored the trace-related code. I do not know if there  
> is a need of modifications.
> 
> Thanks to Johannes Schlatow who gave me the idea of grappling with
> this topic.

I'd love to hear the backstory about this someday. :-)

For integrating your work upstream, I have just opened the following issue:

  https://github.com/genodelabs/genode/issues/2150

Thank you again for submitting your contribution!

Cheers
Norman

-- 
Dr.-Ing. Norman Feske
Genode Labs

http://www.genode-labs.com · http://genode.org

Genode Labs GmbH · Amtsgericht Dresden · HRB 28424 · Sitz Dresden
Geschäftsführer: Dr.-Ing. Norman Feske, Christian Helmuth