Don't call g_variant_store() with NULL

[jlebon]

A GVariant may require 0 bytes to serialize. In which case, data_size will be 0 and we'll call g_malloc0 (0) which returns NULL.

The data argument of g_variant_store is declared as non-nullable, so we shouldn't pass it NULL. In older glib versions, this wasn't explicitly checked and it still transparently worked for our case because there was nothing to actually serialize. Starting from v2.84.1, a precondition was added:

https://gitlab.gnome.org/GNOME/glib/-/commit/39c05b13123b12622ee2c93c170dbf20b573f6ac

Let's respect the API here and stop calling the function if it's not needed.

Note that g_variant_new_from_data() supports being passed NULL and 0 for the data and size arguments.

[jlebon]

See also discussion in https://gitlab.gnome.org/GNOME/glib/-/issues/3675.

[sgallagher]

This patch and #624 seem to disagree about what the input value is. Do we have a non-NULL GVariant with zero-length data, or do we have a NULL GVariant?

[jlebon]

This patch and #624 seem to disagree about what the input value is. Do we have a non-NULL GVariant with zero-length data, or do we have a NULL GVariant?

Missed that. Replied there.

[ppisar]

Excuse me, where is "the data argument of g_variant_store declared as non-nullable"? My glib/gvariant.h from 2.84.1 does not mention it as well as the https://docs.gtk.org/glib/method.Variant.store.html documentation.

The modulemd_variant_deep_copy() is a private function and it is only used when setting XMD for a module, e.g with modulemd_module_stream_v2_set_xmd(). I feel this fix needs to be accompanied with a test.

[jlebon]

Excuse me, where is "the data argument of g_variant_store declared as non-nullable"? My glib/gvariant.h from 2.84.1 does not mention it as well as the docs.gtk.org/glib/method.Variant.store.html documentation.

Hmm, I'm not sure if's supposed to be rendered on that docs page, but the annotation is definitely there: https://gitlab.gnome.org/GNOME/glib/blob/490438c41624d7753f17d3c6e3eb74bcdea1c68b/glib/gvariant-core.c#L1397. And looks like it's been there for quite a while AFAICT.

The modulemd_variant_deep_copy() is a private function and it is only used when setting XMD for a module, e.g with modulemd_module_stream_v2_set_xmd(). I feel this fix needs to be accompanied with a test.

That's reasonable. I don't have the cycles to add a test for this unfortunately. The way I verified this is simply that it no longer logs the assertion failure (and e.g. dnf module list still worked fine).

[ppisar]

Thanks for the explanation. I verified your fix and pushed a test on top of this commit. I will merge it once CI finishes.