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 ------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik _______________________________________________ genode-main mailing list genode-main@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/genode-main
