qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Aleksandar Rikalo" <arikalo@wavecomp.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"David Gibson" <david@gibson.dropbear.id.au>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"Lidong Chen" <lidong.chen@oracle.com>,
	"darren.kenny@oracle.com" <darren.kenny@oracle.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Aleksandar Markovic" <amarkovic@wavecomp.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Richard Henderson" <rth@twiddle.net>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	"f4bug@amsat.org" <f4bug@amsat.org>
Subject: Re: [Qemu-devel] [PATCH] sd: Fix out-of-bounds assertions
Date: Thu, 11 Apr 2019 13:45:35 +0100	[thread overview]
Message-ID: <20190411124535.GS3641@redhat.com> (raw)
In-Reply-To: <87v9zkzqb8.fsf@dusky.pond.sub.org>

On Thu, Apr 11, 2019 at 02:20:27PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, Apr 09, 2019 at 11:37:44AM +0200, Philippe Mathieu-Daudé wrote:
> [...]
> >> I remember this post from Daniel where he suggests sticking to GLib
> >> G_N_ELEMENTS() (which looks similar to your suggestion):
> >> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02676.html
> >> 
> >> $ git grep G_N_ELEMENTS|wc -l
> >> 125
> >> $ git grep ARRAY_SIZE|wc -l
> >> 939
> >> 
> >> Now it is not obvious to me to understand which GLib API we are
> >> encouraged to use and which ones we shouldn't.
> >
> > We have a bunch of duplication that is essentially a historical
> > artifact from before we used GLib in QEMU. IMHO, if GLib provides
> > something that is equivalent with no functional downside that
> > matters to QEMU, then there's no reason to keep QEMU's duplicate.
> >
> > IOW, I would always prefer GLib unless there's a compelling reason
> > not to in order to minimize what we maintain ourselves
> 
> Without a tree-wide sweep, we won't ever stop maintaining ARRAY_SIZE().

In this case it is a pretty trivial search & replace to do. The main
hard bit would be syncing with various maintainer trees. Global cleanups
are more work if you need to feed patches in via several different trees,
as opposed to one tree-wide series.

> As long as we maintain it anyway, I'd prefer it over G_N_ELEMENTS()
> myself, because I consider latter name in poor taste: elements of
> *what*?

elements of the variable that you are passing into the macro.

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

WARNING: multiple messages have this Message-ID (diff)
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Aleksandar Rikalo" <arikalo@wavecomp.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"f4bug@amsat.org" <f4bug@amsat.org>,
	"Lidong Chen" <lidong.chen@oracle.com>,
	"darren.kenny@oracle.com" <darren.kenny@oracle.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Aleksandar Markovic" <amarkovic@wavecomp.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Richard Henderson" <rth@twiddle.net>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	"David Gibson" <david@gibson.dropbear.id.au>
Subject: Re: [Qemu-devel] [PATCH] sd: Fix out-of-bounds assertions
Date: Thu, 11 Apr 2019 13:45:35 +0100	[thread overview]
Message-ID: <20190411124535.GS3641@redhat.com> (raw)
Message-ID: <20190411124535.RfYgoR4NPGkPJNCQQrynTOJrryccXCryL31y423o9Rc@z> (raw)
In-Reply-To: <87v9zkzqb8.fsf@dusky.pond.sub.org>

On Thu, Apr 11, 2019 at 02:20:27PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, Apr 09, 2019 at 11:37:44AM +0200, Philippe Mathieu-Daudé wrote:
> [...]
> >> I remember this post from Daniel where he suggests sticking to GLib
> >> G_N_ELEMENTS() (which looks similar to your suggestion):
> >> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02676.html
> >> 
> >> $ git grep G_N_ELEMENTS|wc -l
> >> 125
> >> $ git grep ARRAY_SIZE|wc -l
> >> 939
> >> 
> >> Now it is not obvious to me to understand which GLib API we are
> >> encouraged to use and which ones we shouldn't.
> >
> > We have a bunch of duplication that is essentially a historical
> > artifact from before we used GLib in QEMU. IMHO, if GLib provides
> > something that is equivalent with no functional downside that
> > matters to QEMU, then there's no reason to keep QEMU's duplicate.
> >
> > IOW, I would always prefer GLib unless there's a compelling reason
> > not to in order to minimize what we maintain ourselves
> 
> Without a tree-wide sweep, we won't ever stop maintaining ARRAY_SIZE().

In this case it is a pretty trivial search & replace to do. The main
hard bit would be syncing with various maintainer trees. Global cleanups
are more work if you need to feed patches in via several different trees,
as opposed to one tree-wide series.

> As long as we maintain it anyway, I'd prefer it over G_N_ELEMENTS()
> myself, because I consider latter name in poor taste: elements of
> *what*?

elements of the variable that you are passing into the macro.

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


  parent reply	other threads:[~2019-04-11 12:57 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-08 19:04 [Qemu-devel] [PATCH] sd: Fix out-of-bounds assertions Lidong Chen
2019-04-08 19:04 ` Lidong Chen
2019-04-08 19:53 ` Marc-André Lureau
2019-04-08 19:53   ` Marc-André Lureau
2019-04-08 21:27 ` Philippe Mathieu-Daudé
2019-04-08 21:27   ` Philippe Mathieu-Daudé
2019-04-08 21:57   ` Lidong Chen
2019-04-08 21:57     ` Lidong Chen
2019-04-09  0:18 ` Li Qiang
2019-04-09  0:18   ` Li Qiang
2019-04-09  5:51 ` Markus Armbruster
2019-04-09  5:51   ` Markus Armbruster
2019-04-09  8:59   ` Aleksandar Markovic
2019-04-09  8:59     ` Aleksandar Markovic
2019-04-09  9:37     ` Philippe Mathieu-Daudé
2019-04-09  9:37       ` Philippe Mathieu-Daudé
2019-04-11 11:52       ` Daniel P. Berrangé
2019-04-11 11:52         ` Daniel P. Berrangé
2019-04-11 12:20         ` Markus Armbruster
2019-04-11 12:20           ` Markus Armbruster
2019-04-11 12:45           ` Daniel P. Berrangé [this message]
2019-04-11 12:45             ` Daniel P. Berrangé
2019-04-11 13:25             ` Markus Armbruster
2019-04-11 13:25               ` Markus Armbruster
2019-04-09  9:40   ` Aleksandar Markovic
2019-04-09  9:40     ` Aleksandar Markovic
2019-04-09  9:48   ` Peter Maydell
2019-04-09  9:48     ` Peter Maydell
2019-04-09 10:39   ` Liam Merwick
2019-04-09 10:39     ` Liam Merwick
2019-04-10 21:49     ` Lidong Chen
2019-04-10 21:49       ` Lidong Chen

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=20190411124535.GS3641@redhat.com \
    --to=berrange@redhat.com \
    --cc=amarkovic@wavecomp.com \
    --cc=arikalo@wavecomp.com \
    --cc=armbru@redhat.com \
    --cc=aurelien@aurel32.net \
    --cc=darren.kenny@oracle.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=f4bug@amsat.org \
    --cc=jasowang@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=lidong.chen@oracle.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).