Skip to contentSkip to conversation

replace functions which are only available in glib-2.24

PATCH In review Olaf Hering 4 participants 4 messages May 15, 2018 – Jun 12, 2018

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 replace functions which are only available in glib-2.24

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")

---

This must go into stable-2.12.

 util/vfio-helpers.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
util/vfio-helpers.c +2 -4
index 006674c916..1d9272efa4 100644
@@ -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. */
Reviewed-by on v2 1/1
#1 lore raw
Quoted 5 lines · Olaf Hering

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.

Quoted 7 lines

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

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

---
This must go into stable-2.12.

In that case please CC: [email protected] (done now).

Quoted 28 lines
 util/vfio-helpers.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
util/vfio-helpers.c +2 -4
index 006674c916..1d9272efa4 100644
@@ -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. */
Reviewed-by
#2 lore raw
Quoted 44 lines · On Tue, 15 May 2018 09:18:52 +0200 Thomas Huth <[email protected]> wrote:
Quoted 5 lines · Olaf Hering

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.

Quoted 7 lines

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

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

---
This must go into stable-2.12.

In that case please CC: [email protected] (done now).

Quoted 26 lines
 util/vfio-helpers.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
util/vfio-helpers.c +2 -4
index 006674c916..1d9272efa4 100644
@@ -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);
 }
 
Quoted 2 lines

util/vfio-helpers.c static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping, · line 582

Quoted 2 lines
util/vfio-helpers.c +0 -0
 /* Check if the mapping list is (ascending) ordered. */

This looks fine to me, so

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?

#3 lore raw

Quoting Cornelia Huck (2018-05-15 03:09:49)

Quoted 52 lines
Quoted 44 lines · On Tue, 15 May 2018 09:18:52 +0200 Thomas Huth <[email protected]> wrote:
Quoted 5 lines · Olaf Hering

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.

Quoted 7 lines

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

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

---
This must go into stable-2.12.

In that case please CC: [email protected] (done now).

Quoted 26 lines
 util/vfio-helpers.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
util/vfio-helpers.c +2 -4
index 006674c916..1d9272efa4 100644
@@ -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);
 }
 
Quoted 2 lines

util/vfio-helpers.c static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping, · line 582

Quoted 2 lines
util/vfio-helpers.c +0 -0
 /* Check if the mapping list is (ascending) ordered. */

This looks fine to me, so

Quoted 8 lines

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