All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@gmail.com>
Cc: "Eduardo Habkost" <ehabkost@redhat.com>,
	QEMU <qemu-devel@nongnu.org>, "Gerd Hoffmann" <kraxel@redhat.com>,
	"Samuel Thibault" <samuel.thibault@ens-lyon.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH 00/18] chardev: QOM cleanups
Date: Fri, 11 Sep 2020 09:10:18 +0100	[thread overview]
Message-ID: <20200911081018.GA1203593@redhat.com> (raw)
In-Reply-To: <CAJ+F1CLLKesMvZo4DJcC=f387d_fN8g--qX8YEQGu8Thf1qotA@mail.gmail.com>

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.

> 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.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2020-09-11  8:11 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10 19:48 [PATCH 00/18] chardev: QOM cleanups Eduardo Habkost
2020-09-10 19:48 ` [PATCH 01/18] chardev: Move PARALLEL_CHARDEV macro to common code Eduardo Habkost
2020-09-10 19:48 ` [PATCH 02/18] chardev: Move ParallelChardev typedef " Eduardo Habkost
2020-09-10 19:48 ` [PATCH 03/18] chardev: Use DECLARE_INSTANCE_CHECKER macro for PARALLEL_CHARDEV Eduardo Habkost
2020-09-10 19:48 ` [PATCH 04/18] chardev: Rename MOUSE_CHARDEV to CHARDEV_MSMOUSE Eduardo Habkost
2020-09-10 19:48 ` [PATCH 05/18] chardev: Rename BAUM_CHARDEV to CHARDEV_BRAILLE Eduardo Habkost
2020-09-10 19:48 ` [PATCH 06/18] chardev: Rename FD_CHARDEV to CHARDEV_FD Eduardo Habkost
2020-09-10 19:48 ` [PATCH 07/18] chardev: Rename MUX_CHARDEV to CHARDEV_MUX Eduardo Habkost
2020-09-10 19:48 ` [PATCH 08/18] chardev: Rename PARALLEL_CHARDEV to CHARDEV_PARALLEL Eduardo Habkost
2020-09-10 19:48 ` [PATCH 09/18] chardev: Rename PTY_CHARDEV to CHARDEV_PTY Eduardo Habkost
2020-09-10 19:48 ` [PATCH 10/18] chardev: Rename RINGBUF_CHARDEV to CHARDEV_RINGBUF Eduardo Habkost
2020-09-10 19:48 ` [PATCH 11/18] chardev: Rename SOCKET_CHARDEV to CHARDEV_SOCKET Eduardo Habkost
2020-09-10 19:48 ` [PATCH 12/18] chardev: Rename SPICE_CHARDEV to CHARDEV_SPICE Eduardo Habkost
2020-09-10 19:48 ` [PATCH 13/18] chardev: Rename TESTDEV_CHARDEV to CHARDEV_TESTDEV Eduardo Habkost
2020-09-10 19:48 ` [PATCH 14/18] chardev: Rename UDP_CHARDEV to CHARDEV_UDP Eduardo Habkost
2020-09-10 19:49 ` [PATCH 15/18] chardev: Rename VC_CHARDEV to CHARDEV_VC Eduardo Habkost
2020-09-10 19:49 ` [PATCH 16/18] chardev: Rename WCTABLET_CHARDEV to CHARDEV_WCTABLET Eduardo Habkost
2020-09-10 19:49 ` [PATCH 17/18] chardev: Rename WIN_CHARDEV to CHARDEV_WIN Eduardo Habkost
2020-09-10 19:49 ` [PATCH 18/18] chardev: Rename WIN_STDIO_CHARDEV to CHARDEV_WIN_STDIO Eduardo Habkost
2020-09-11  8:07 ` [PATCH 00/18] chardev: QOM cleanups Marc-André Lureau
2020-09-11  8:10   ` Daniel P. Berrangé [this message]
2020-09-11  8:19     ` Marc-André Lureau
2020-09-11  8:32       ` Daniel P. Berrangé
2020-09-11 13:50     ` Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200911081018.GA1203593@redhat.com \
    --to=berrange@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=samuel.thibault@ens-lyon.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.