[sdbusplus] To generate client header (was: Re: To generate a common header for public information of interfaces)

[Lei YU]

On Tue, Feb 25, 2020 at 11:59 PM Patrick Williams <patrick at stwcx.xyz> wrote:
>
> On Tue, Feb 25, 2020 at 04:22:40PM +0800, Lei YU wrote:
> > > Examples of poor code are not a use case.  Both of these examples are
> > > trivially converted to convert<enum>ToString APIs.  We should do that
> > > rather than facilitate unmaintainable code.
> >
> > My concern to use "convertXXX" APIs is that it requires the client
> > code to link the server code, where logically all it needs is the enum
> > string.
>
> I'm not sure if you're referring to an API level issue or ABI level
> issue.  Almost anything at an API level can be dealt with by headers (as
> you're proposing to generate) and light-weight indirects.  Being worried
> about linking at an ABI level seems like a premature optimization (we
> only make libphosphor-dbus.so today anyhow).

I am referring to the code structure. Clean code should include the
headers it really needs, and should not include unnecessary headers.
If clientA's code needs to talk to serverB, it ideally includes
clientB.hpp instead of serverB.hpp.
So the proposal here is to make sdbusplus generate the client header
to be used by client code.

>
> > This is exactly why I propose to provide enum strings in by the client header.
> > Currently, client code (poorly) uses "hard-coded" strings directly. If
> > we provide the constexpr strings in the client header, the client code
> > could be "refacted" to use the definitions from the client header.
> > Then sdbusplus is freely to update the string format without breaking
> > client code.
>
> Why not just fix them to use enums properly?  There is zero reason for
> applications to be dealing in string manipulation for these.
>

It still does not resolve the case when user *does* want to use a enum
string as constexpr.
If sdbusplus could provide constexpr function to convert enum to
string, it will be fine.
Do you think if that's possible?

> > > So maybe we call it "client" rather than "common"?  Common implies that
> > > there are header files that are okay to import under any case and I
> > > don't see that to be the case with what you're proposing we define.
> > >
> >
> > Yeah, agreed.
>
> :thumbsup:
>
> > > Again, I don't have any issue with the interface names.  I do take issue
> > > with the enumeration strings because they shouldn't ever be used outside
> > > of sdbusplus (or a similar dbus binding).
> > >
> >
> > For the interface names, the patch is updated at:
> > https://gerrit.openbmc-project.xyz/c/openbmc/sdbusplus/+/29404
> > Let's treat it as a start point of client code.
>
> I'll take a look soon.
>
> --
> Patrick Williams