[sdbusplus] To generate a common header for public information of interfaces

Lei YU mine260309 at gmail.com
Mon Feb 24 14:25:46 AEDT 2020


> > The idea of my proposal here is not to use "convertXXX" functions,
> > instead, it is to provide the constexpr strings that could be used by
> > client code.
> > E.g. the client code does not need to call any function, but directly use:
> >
> > * xyz::...::server::MyServer::interface, instead of a user-defined string
> >   for the interface.
> > * xyz::...::MyServer::MyEnum::Enum1, instead of a user-defined string for
> >   the enum.
>
> I don't see any reason any code should ever directly utilize the enum
> strings.  We generate code already to safely convert them to and from
> real C++ enum types.  Why would you want to use string comparison
> instead?
>
> Can you elaborate on why we should be enabling this usage pattern?

There are cases for a service to use a "enum string" directly instead
of getting the string from enum by `convertForMessage()`
E.g.
* https://github.com/openbmc/phosphor-bmc-code-mgmt/blob/master/activation.hpp#L43
* https://github.com/openbmc/phosphor-host-ipmid/blob/master/chassishandler.cpp#L218

Some of the cases could be changed to use `convertForMessage()`, but
if one does want to directly use that, or use it as constexpr, why not
provide it directly?

>
> > Yup, the implementation is actually part of the "client" header, we
> > could rename the "common" to "client".
> > But preferably, we could combine all the client header into a single
> > header, which makes it easier for the client code to use.
> > If a client needs to set an enum property to a service, it then does
> > not have to include the server header nor the specific client header,
> > but include a single header.
> > Anyway, this part is done in phosphor-dbus-interface.
> >
> > So we could say that sdbusplus generates part of the "client" header
> > instead of "common" header.
>
> Agree, but we don't even need this code now if people were just
> including the server header files (and there are many examples of
> clients doing exactly this).  If we're not going to create client
> headers now, this seems like wasted effort just for some slight syntax
> improvement.
>
> We could simply symlink <xyz/openbmc_project/foo/server.hpp> to
> <xyz/openbmc_project/foo/client.hpp> and add a `namespace
> xyz::openbmc_project::foo::client = xyz::openbmc_project:server;` and
> get the same syntactic enhancement without nearly as much work.
>

I do not think it's good behavior for a client to include another
server's heaader files.
E.g. if I am writing client code and want to set a property on another
service (e.g. foo.bar), I do not really want to include a
"foo/bar/server.hpp", because it's not part of my client.
What I want is some constexpr strings, like interface name, property
name, enum string, etc.

Ideally, sdbusplus would generate the full client binding code. But we
do not have that now.
So I am proposing to move a bit forward to generate a little client
code that would benefit the client's usage. The client won't have to
write the full string manually anymore.


> --
> Patrick Williams


More information about the openbmc mailing list