Re: [PATCH 00/18] chardev: QOM cleanups

[Eduardo Habkost]

On Fri, Sep 11, 2020 at 09:10:18AM +0100, Daniel P. Berrangé wrote:
> On Fri, Sep 11, 2020 at 12:07:27PM +0400, Marc-André Lureau wrote:
> > Hi
> > 
> > On Thu, Sep 10, 2020 at 11:50 PM Eduardo Habkost <ehabkost@redhat.com>
> > wrote:
> > 
> > > Some chardev QOM cleanup patches had to be dropped from my queue
> > > due to build erros introduced by code movements across ifdef
> > > boundaries at char-parallel.c.  This series redo the changes from
> > > those patches, but the macro renames are now a little different:
> > >
> > > In this version I have decided to rename the type checking macros
> > > from *_CHARDEV to CHARDEV_* instead of renaming tye
> > > TYPE_CHARDEV_* constants to TYPE_*_CHARDEV, to make the
> > > identifiers actually match the QOM type name strings
> > > ("chardev-*").
> > >
> > 
> > Sounds reasonable to me, but it loses the matching with the
> > structure/object type name though.
> > 
> > - MuxChardev *d = MUX_CHARDEV(s);
> > + MuxChardev *d = CHARDEV_MUX(s);
> > 
> > I have a preference for the first. Unless we rename all the chardev types
> > too...
> 
> I tend to think the structs should be renamed too - I've always considerd
> them to be backwards.

FWIW, "MuxChardev" sounds better to me.  Not a big deal, though.

(Also, I am not planning to touch any struct names for the sake
of the new QOM declaration/definition macros.  Renaming the type
checking functions is enough churn.)

> 
> > Imho, the QOM type name is mostly an internal detail, the C type name is
> > dominant in the code.
> 
> Err it is the reverse. The QOM type name is exposed public API via QOM
> commands, while the C struct names are a internal private detail.

I agree with Marc-André here.  The C code is not just a detail.
Code needs be easy to read, change and refactor.  I'd really
prefer to not have the external public API forcing a specific
internal naming style.

We have at least one case where it's probably going to be
impossible to keep an exact match between the QOM type and type
checker functions: "accel"/ACCEL.
CAFEAcA9WEjne5TfwggVWPuBprkRs-a2-iNc43Xa_jBamaf9t8A@mail.gmail.com/">https://lore.kernel.org/qemu-devel/CAFEAcA9WEjne5TfwggVWPuBprkRs-a2-iNc43Xa_jBamaf9t8A@mail.gmail.com/

-- 
Eduardo