diff mbox

[v2] replace functions which are only available in glib-2.24

Message ID 20180515063128.7040-1-olaf@aepfle.de (mailing list archive)
State New, archived
Headers show

Commit Message

Olaf Hering May 15, 2018, 6:31 a.m. UTC
Currently the minimal supported version of glib is 2.22.
Since testing is done with a glib that claims to be 2.22, but in fact
has APIs from newer version of glib, this bug was not caught during
submit of the patch referenced below.

Replace g_realloc_n, which is available only since 2.24, with g_renew.

Fixes commit 418026ca43 ("util: Introduce vfio helpers")

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---

 This must go into stable-2.12.

 util/vfio-helpers.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Thomas Huth May 15, 2018, 7:18 a.m. UTC | #1
On 15.05.2018 08:31, Olaf Hering wrote:
> Currently the minimal supported version of glib is 2.22.
> Since testing is done with a glib that claims to be 2.22, but in fact
> has APIs from newer version of glib, this bug was not caught during
> submit of the patch referenced below.
> 
> Replace g_realloc_n, which is available only since 2.24, with g_renew.
> 
> Fixes commit 418026ca43 ("util: Introduce vfio helpers")
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  This must go into stable-2.12.

In that case please CC: qemu-stable@nongnu.org (done now).

Reviewed-by: Thomas Huth <thuth@redhat.com>

>  util/vfio-helpers.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> index 006674c916..1d9272efa4 100644
> --- a/util/vfio-helpers.c
> +++ b/util/vfio-helpers.c
> @@ -522,8 +522,7 @@ static IOVAMapping *qemu_vfio_add_mapping(QEMUVFIOState *s,
>  
>      assert(index >= 0);
>      s->nr_mappings++;
> -    s->mappings = g_realloc_n(s->mappings, sizeof(s->mappings[0]),
> -                              s->nr_mappings);
> +    s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings);
>      insert = &s->mappings[index];
>      shift = s->nr_mappings - index - 1;
>      if (shift) {
> @@ -577,8 +576,7 @@ static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping,
>      memmove(mapping, &s->mappings[index + 1],
>              sizeof(s->mappings[0]) * (s->nr_mappings - index - 1));
>      s->nr_mappings--;
> -    s->mappings = g_realloc_n(s->mappings, sizeof(s->mappings[0]),
> -                              s->nr_mappings);
> +    s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings);
>  }
>  
>  /* Check if the mapping list is (ascending) ordered. */
>
Cornelia Huck May 15, 2018, 8:09 a.m. UTC | #2
On Tue, 15 May 2018 09:18:52 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 15.05.2018 08:31, Olaf Hering wrote:
> > Currently the minimal supported version of glib is 2.22.
> > Since testing is done with a glib that claims to be 2.22, but in fact
> > has APIs from newer version of glib, this bug was not caught during
> > submit of the patch referenced below.
> > 
> > Replace g_realloc_n, which is available only since 2.24, with g_renew.
> > 
> > Fixes commit 418026ca43 ("util: Introduce vfio helpers")
> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > ---
> >  This must go into stable-2.12.  
> 
> In that case please CC: qemu-stable@nongnu.org (done now).
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
> >  util/vfio-helpers.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> > index 006674c916..1d9272efa4 100644
> > --- a/util/vfio-helpers.c
> > +++ b/util/vfio-helpers.c
> > @@ -522,8 +522,7 @@ static IOVAMapping *qemu_vfio_add_mapping(QEMUVFIOState *s,
> >  
> >      assert(index >= 0);
> >      s->nr_mappings++;
> > -    s->mappings = g_realloc_n(s->mappings, sizeof(s->mappings[0]),
> > -                              s->nr_mappings);
> > +    s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings);
> >      insert = &s->mappings[index];
> >      shift = s->nr_mappings - index - 1;
> >      if (shift) {
> > @@ -577,8 +576,7 @@ static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping,
> >      memmove(mapping, &s->mappings[index + 1],
> >              sizeof(s->mappings[0]) * (s->nr_mappings - index - 1));
> >      s->nr_mappings--;
> > -    s->mappings = g_realloc_n(s->mappings, sizeof(s->mappings[0]),
> > -                              s->nr_mappings);
> > +    s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings);
> >  }
> >  
> >  /* Check if the mapping list is (ascending) ordered. */
> >   

This looks fine to me, so

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

But this leads to another question: What do we do if we want a fix only
in stable? Hypothetical scenario: Same parameters as for this patch
(glib version bump, newer function sneaked in), but the older function
that could be used in stable is clearly worse (IOW, we don't want it in
the new version). Do we have a process to get a change only into stable?
Michael Roth June 12, 2018, 10:21 p.m. UTC | #3
Quoting Cornelia Huck (2018-05-15 03:09:49)
> On Tue, 15 May 2018 09:18:52 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 15.05.2018 08:31, Olaf Hering wrote:
> > > Currently the minimal supported version of glib is 2.22.
> > > Since testing is done with a glib that claims to be 2.22, but in fact
> > > has APIs from newer version of glib, this bug was not caught during
> > > submit of the patch referenced below.
> > > 
> > > Replace g_realloc_n, which is available only since 2.24, with g_renew.
> > > 
> > > Fixes commit 418026ca43 ("util: Introduce vfio helpers")
> > > 
> > > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > > ---
> > >  This must go into stable-2.12.  
> > 
> > In that case please CC: qemu-stable@nongnu.org (done now).
> > 
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > 
> > >  util/vfio-helpers.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> > > index 006674c916..1d9272efa4 100644
> > > --- a/util/vfio-helpers.c
> > > +++ b/util/vfio-helpers.c
> > > @@ -522,8 +522,7 @@ static IOVAMapping *qemu_vfio_add_mapping(QEMUVFIOState *s,
> > >  
> > >      assert(index >= 0);
> > >      s->nr_mappings++;
> > > -    s->mappings = g_realloc_n(s->mappings, sizeof(s->mappings[0]),
> > > -                              s->nr_mappings);
> > > +    s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings);
> > >      insert = &s->mappings[index];
> > >      shift = s->nr_mappings - index - 1;
> > >      if (shift) {
> > > @@ -577,8 +576,7 @@ static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping,
> > >      memmove(mapping, &s->mappings[index + 1],
> > >              sizeof(s->mappings[0]) * (s->nr_mappings - index - 1));
> > >      s->nr_mappings--;
> > > -    s->mappings = g_realloc_n(s->mappings, sizeof(s->mappings[0]),
> > > -                              s->nr_mappings);
> > > +    s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings);
> > >  }
> > >  
> > >  /* Check if the mapping list is (ascending) ordered. */
> > >   
> 
> This looks fine to me, so
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 
> But this leads to another question: What do we do if we want a fix only
> in stable? Hypothetical scenario: Same parameters as for this patch
> (glib version bump, newer function sneaked in), but the older function
> that could be used in stable is clearly worse (IOW, we don't want it in
> the new version). Do we have a process to get a change only into stable?
> 

I've answered in the form of a patch :)

  "docs: add details regarding submitting stable-specific patches"

but here's a recent example:

  https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg04999.html
diff mbox

Patch

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 006674c916..1d9272efa4 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -522,8 +522,7 @@  static IOVAMapping *qemu_vfio_add_mapping(QEMUVFIOState *s,
 
     assert(index >= 0);
     s->nr_mappings++;
-    s->mappings = g_realloc_n(s->mappings, sizeof(s->mappings[0]),
-                              s->nr_mappings);
+    s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings);
     insert = &s->mappings[index];
     shift = s->nr_mappings - index - 1;
     if (shift) {
@@ -577,8 +576,7 @@  static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping,
     memmove(mapping, &s->mappings[index + 1],
             sizeof(s->mappings[0]) * (s->nr_mappings - index - 1));
     s->nr_mappings--;
-    s->mappings = g_realloc_n(s->mappings, sizeof(s->mappings[0]),
-                              s->nr_mappings);
+    s->mappings = g_renew(IOVAMapping, s->mappings, s->nr_mappings);
 }
 
 /* Check if the mapping list is (ascending) ordered. */