return g_variant_new_maybe for NULL variants
#624
Conversation
From glib 2.84.1, `g_variant_store` will not accept `NULL` values instead failing an assertion and throwing a critical error. In the case where `variant` == `NULL`, instead we can wrap and return a `g_variant_new_maybe` with a child of `NULL` to achieve the same effective result. Closes: fedora-modularity#623 Signed-off-by: Matt Jolly <kangie@gentoo.org>
| modulemd_variant_deep_copy (GVariant *variant) | ||
| { | ||
| // g_variant_store will not accept NULL from 2.84.1 | ||
| if (variant == NULL) | ||
| { | ||
| return g_variant_ref_sink (g_variant_new_maybe (G_VARIANT_TYPE_VARIANT, NULL)); | ||
| } | ||
|
|
There was a problem hiding this comment.
If we're passeed NULL for input to a copy function, shouldn't we just return NULL instead? There's obviously nothing to actually copy, so creating an empty variant seems odd.
It's also entirely possible that we need to figure out why it's getting passed NULL in the first place. Ticket #623 suggests that interacting with certain repos causes this, so could you figure out how to recreate the problem (ideally in the test suite)?
|
Ahh I had missed this patch and opened #625. Note the patch is not correct. The variant itself is valid. It's simply that it has no children. Because GVariants don't include type information when serialized (they're expected to be provided OOB), there is nothing to serialize. The type does survive the deep copy though as written in #625. |
|
Let's use #625 |
3 participants
From glib 2.84.1,
g_variant_storewill not acceptNULLvalues instead failing an assertion and throwing a critical error.In the case where
variant==NULL, instead we can wrap and return ag_variant_new_maybewith a child ofNULLto achieve the same effective result.Closes: #623