include/hw/mem/nvdimm.h | 1 + include/qom/object.h | 25 ++++++++++++++++--------- hw/acpi/ich9.c | 2 +- hw/acpi/nvdimm.c | 2 +- hw/acpi/piix4.c | 2 +- hw/i386/pc.c | 8 +++----- hw/mem/pc-dimm.c | 2 +- 7 files changed, 24 insertions(+), 18 deletions(-)
Hi, This is a readability suggestion. See the first patch for the doc change, and the second patch for an example of usage and readability improvement. thanks Marc-André Lureau (2): object: recommend a few type check macros nvdimm: use IS_NVDIMM macro include/hw/mem/nvdimm.h | 1 + include/qom/object.h | 25 ++++++++++++++++--------- hw/acpi/ich9.c | 2 +- hw/acpi/nvdimm.c | 2 +- hw/acpi/piix4.c | 2 +- hw/i386/pc.c | 8 +++----- hw/mem/pc-dimm.c | 2 +- 7 files changed, 24 insertions(+), 18 deletions(-) -- 2.19.0
On Fri, 21 Sep 2018 15:13:30 +0400 Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > Hi, > > This is a readability suggestion. See the first patch for the doc > change, and the second patch for an example of usage and readability > improvement. Looks like a nice improvement, but could you conversion tree wide so we end up with a consistent way to cast across the tree. Probably it would result in negative diffstat and demonstrate usefulness of the simplification in numbers. > thanks > > Marc-André Lureau (2): > object: recommend a few type check macros > nvdimm: use IS_NVDIMM macro > > include/hw/mem/nvdimm.h | 1 + > include/qom/object.h | 25 ++++++++++++++++--------- > hw/acpi/ich9.c | 2 +- > hw/acpi/nvdimm.c | 2 +- > hw/acpi/piix4.c | 2 +- > hw/i386/pc.c | 8 +++----- > hw/mem/pc-dimm.c | 2 +- > 7 files changed, 24 insertions(+), 18 deletions(-) >
Hi On Mon, Sep 24, 2018 at 1:30 PM Igor Mammedov <imammedo@redhat.com> wrote: > > On Fri, 21 Sep 2018 15:13:30 +0400 > Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > > > Hi, > > > > This is a readability suggestion. See the first patch for the doc > > change, and the second patch for an example of usage and readability > > improvement. > Looks like a nice improvement, > but could you conversion tree wide so we end up with a consistent > way to cast across the tree. > Probably it would result in negative diffstat and demonstrate > usefulness of the simplification in numbers. It doesn't result in negative diffstat, since often a one-liner is replaced by a 2-liner macro ;) However, the resulting code is more pleasant to read imho. I attached the whole diff here, the branch with the 49 patches split-series is here: https://github.com/elmarco/qemu/commits/object Let me know what you think > > > thanks > > > > Marc-André Lureau (2): > > object: recommend a few type check macros > > nvdimm: use IS_NVDIMM macro > > > > include/hw/mem/nvdimm.h | 1 + > > include/qom/object.h | 25 ++++++++++++++++--------- > > hw/acpi/ich9.c | 2 +- > > hw/acpi/nvdimm.c | 2 +- > > hw/acpi/piix4.c | 2 +- > > hw/i386/pc.c | 8 +++----- > > hw/mem/pc-dimm.c | 2 +- > > 7 files changed, 24 insertions(+), 18 deletions(-) > > > > -- Marc-André Lureau
Hi On Mon, Oct 1, 2018 at 5:47 PM Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > Hi > > On Mon, Sep 24, 2018 at 1:30 PM Igor Mammedov <imammedo@redhat.com> wrote: > > > > On Fri, 21 Sep 2018 15:13:30 +0400 > > Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > > > > > Hi, > > > > > > This is a readability suggestion. See the first patch for the doc > > > change, and the second patch for an example of usage and readability > > > improvement. > > Looks like a nice improvement, > > but could you conversion tree wide so we end up with a consistent > > way to cast across the tree. > > Probably it would result in negative diffstat and demonstrate > > usefulness of the simplification in numbers. > > It doesn't result in negative diffstat, since often a one-liner is > replaced by a 2-liner macro ;) > > However, the resulting code is more pleasant to read imho. > > I attached the whole diff here, the branch with the 49 patches > split-series is here: https://github.com/elmarco/qemu/commits/object > > Let me know what you think So after doing that conversion work, what do we do? > > > > > > thanks > > > > > > Marc-André Lureau (2): > > > object: recommend a few type check macros > > > nvdimm: use IS_NVDIMM macro > > > > > > include/hw/mem/nvdimm.h | 1 + > > > include/qom/object.h | 25 ++++++++++++++++--------- > > > hw/acpi/ich9.c | 2 +- > > > hw/acpi/nvdimm.c | 2 +- > > > hw/acpi/piix4.c | 2 +- > > > hw/i386/pc.c | 8 +++----- > > > hw/mem/pc-dimm.c | 2 +- > > > 7 files changed, 24 insertions(+), 18 deletions(-) > > > > > > > > > > -- > Marc-André Lureau -- Marc-André Lureau
On Tue, Oct 30, 2018 at 08:37:47PM +0400, Marc-André Lureau wrote: > Hi > > On Mon, Oct 1, 2018 at 5:47 PM Marc-André Lureau > <marcandre.lureau@gmail.com> wrote: > > > > Hi > > > > On Mon, Sep 24, 2018 at 1:30 PM Igor Mammedov <imammedo@redhat.com> wrote: > > > > > > On Fri, 21 Sep 2018 15:13:30 +0400 > > > Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > > > > > > > Hi, > > > > > > > > This is a readability suggestion. See the first patch for the doc > > > > change, and the second patch for an example of usage and readability > > > > improvement. > > > Looks like a nice improvement, > > > but could you conversion tree wide so we end up with a consistent > > > way to cast across the tree. > > > Probably it would result in negative diffstat and demonstrate > > > usefulness of the simplification in numbers. > > > > It doesn't result in negative diffstat, since often a one-liner is > > replaced by a 2-liner macro ;) > > > > However, the resulting code is more pleasant to read imho. > > > > I attached the whole diff here, the branch with the 49 patches > > split-series is here: https://github.com/elmarco/qemu/commits/object > > > > Let me know what you think > > So after doing that conversion work, what do we do? I say submit it, as I'm in favour of the readability of the macro and like the alignment with gobject paradigm. 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 :|
© 2016 - 2025 Red Hat, Inc.