Support for names of method and signal arguments in DBus introspection

[mkaranki]

sdbusplus does not add names of the arguments of methods and parameters to the D-Bus introspection document.

systemd sd-bus has added support for that in v242 release. See systemd/systemd#1564

I did a quick try out how the support could be implemented. See
mkaranki@27e6a8d

I could spend few more moments to polish that to perhaps get it merged, but would like to receive some advice regarding the at least the following things:

1. The vtable API compatibility. Is is preferred to add vtable::method_n and vtable::signal_n, i.e. functions defining method or signal with argument names, to retain the old API, or would it be better to add the name parameters to the vtable::method and vtable::signal functions? Is there value of maintaining the old API or supporting genuinely unnamed arguments?

2. vtable::method_n arguments. sdbus' SD_BUS_METHOD_WITH_NAMES macro is having separate arguments for parameter names and return value names. Those seem to be expected to be string literals and are concatenated to be a single names array, where names are delimited by nil characters. Would it be better to keep the vtable::method_n to have a single names argument, which is rather simple, or have in and out names separated just like SD_BUS_METHOD_WITH_NAMES has it, and then somehow combine them to be one list? I am unsure could the latter be constexpr or not / how to do that without spending extra memory for that.

[williamspatrick]

@mkaranki - Thanks for starting on this work. It wasn't something I was aware of before you pointed it out.

It looks like you have are really terrific start on this!

Since I don't recognize your name, a few items for your awareness:

• We currently do our development on our Gerrit server (gerrit.openbmc-project.xyz) and don't accept pull requests.
• The OpenBMC project requires a CLA for submissions (unfortunately, in my opinion).
• There was some work underway to refactor a few of the python templates (https://gerrit.openbmc-project.xyz/c/openbmc/sdbusplus/+/38344). I'm going to merge this now I think so you'll have it available.

Thinking through it a little bit, one flaw we might have is that return values we don't require to be named in the sdbus++ YAML. We could start enforcing that and I may have to clean up some of the YAML in openbmc/phosphor-dbus-interfaces. I'd be fine with requiring names on those. We could also generate a fake name like Unnamed<N>.

The vtable API compatibility.

I don't think there are any cases of people using vtable::method / vtable:signal interfaces directly, but if there are we can clean them up. As long as the sdbus++ generators are shifted to the new API I think that will cover 99% of users. I'd be fine deprecating the "old" interface at that point.

vtable::method_n arguments.

Have you looked at SD_BUS_METHOD_WITH_ARGS and SD_BUS_SIGNAL_WITH_ARGS? I suggest using those instead since the man page now says:

               Prefer using SD_BUS_SIGNAL_WITH_ARGS() over these macros as it
               allows specifying argument types and names next to each other
               which is less error-prone than first specifying all argument
               types followed by specifying all argument names.

In that case it looks like parameters are passed to the WITH_ARGS macros as type/name pairs.

[mkaranki]

Have you looked at SD_BUS_METHOD_WITH_ARGS and SD_BUS_SIGNAL_WITH_ARGS?

I did not know about those. Their API indeed looks nicer, but I've been thinking few days now how they could be used in sdbusplus.

The main issue with them I think is that it is actually a variadic macro (https://github.com/systemd/systemd/blob/301e7cd047c8d07715d5dc37f713e8aa031581b4/src/systemd/sd-bus-vtable.h#L287). Thus implementing vtable::method as a proper, type safe C++ (constexpr) method that calls the macro with the correct number of arguments seems to not to be really possible. Or at least it would require defining an own overloaded function for each number of arguments. If I understand the macro implementation of SD_BUS_METHOD_WITH_ARGS correctly, it is supporting up to 50 arguments and return values. To fully support the same 50 * 50 = 2500 overloaded C++ constexpr methods would be required. That's quite a lot.

The alternatives so far I've been thinking could be to generate the needed vtable::method and vtable::signal code with mako templates based on the yaml file, or then just simply omit using all kinds of the constexpr vtable::method alike "type safety layers" and call directly the SD_BUS_METHOD_WITH_ARGS macro from the sdbus++ generated code. The generated vtable would look something like:

const vtable::vtable_t xyz::_vtable[] = {
    vtable::start(),
    SD_BUS_METHOD_WITH_ARGS("Method1",
                             SD_BUS_ARGS(std::get<0>(details::xyz::_param_Method1[0])
                                         std::get<1>(details::xyz::_param_Method1[0]),
                                         std::get<0>(details::xyz::_param_Method1[1]),
                                         std::get<1>(details::xyz::_param_Method1[1])),
                             SD_BUS_RESULT(std::get<0>(details::xyz::_return_Method1[0]),
                                           std::get<1>(details::xyz::_return_Method1[0])),
                             _callback_Method1,
                             flags),
							 
    SD_BUS_SIGNAL_WITH_ARGS("Signal1",
                            SD_BUS_NO_ARGS,
                            flags),                        
    vtable::end()
};

[williamspatrick]

Looking at the implementation of SD_BUS_METHOD_WITH_ARGS, it seems to be wrapper around SD_BUS_METHOD_WITH_NAMES with a whole lot of pre-processor magic to concatenate strings. I wonder if we could use a C++11 variadic template with maybe std::string_view to perform similar concatenation and call SD_BUS_METHOD_WITH_NAMES directly.