From a8eed03a1974c285cb7ddf550c7385a4783bb97d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 1/7] remote: Fix gcc `-fanalyzer` warning In general, we're probably going to need to change most of our `g_return_if_fail` to `g_assert`. The analyzer flags that the function can return `NULL`, but the caller isn't prepared for this. In practice, let's abort. --- src/libostree/ostree-remote.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-remote.c b/src/libostree/ostree-remote.c index 2b068e15..93c1a328 100644 --- a/src/libostree/ostree-remote.c +++ b/src/libostree/ostree-remote.c @@ -65,8 +65,8 @@ ostree_remote_new_dynamic (const gchar *name, { OstreeRemote *remote; - g_return_val_if_fail (name != NULL && *name != '\0', NULL); - g_return_val_if_fail (refspec_name == NULL || *refspec_name != '\0', NULL); + g_assert (name != NULL && *name != '\0'); + g_assert (refspec_name == NULL || *refspec_name != '\0'); remote = g_slice_new0 (OstreeRemote); remote->ref_count = 1; From 9a7f9c2095a1fe834793978250be402bfb8237f6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 2/7] deployment: Fix gcc `-fanalyzer` warning In general, we're probably going to need to change most of our `g_return_if_fail` to `g_assert`. The analyzer flags that the function can return `NULL`, but the caller isn't prepared for this. In practice, let's abort. --- src/libostree/ostree-deployment.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-deployment.c b/src/libostree/ostree-deployment.c index 558434de..6397d786 100644 --- a/src/libostree/ostree-deployment.c +++ b/src/libostree/ostree-deployment.c @@ -374,9 +374,9 @@ ostree_deployment_new (int index, OstreeDeployment *self; /* index may be -1 */ - g_return_val_if_fail (osname != NULL, NULL); - g_return_val_if_fail (csum != NULL, NULL); - g_return_val_if_fail (deployserial >= 0, NULL); + g_assert (osname != NULL); + g_assert (csum != NULL); + g_assert (deployserial >= 0); /* We can have "disconnected" deployments that don't have a bootcsum/serial */ From 520b45afdd97cab47d55aecab1407a857a7008df Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 3/7] sysroot: Fix gcc `-fanalyzer` warning In general, we're probably going to need to change most of our `g_return_if_fail` to `g_assert`. The analyzer flags that the function can return `NULL`, but the caller isn't prepared for this. In practice, let's abort. --- src/libostree/ostree-sysroot.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index be5306b7..04432cbc 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1235,12 +1235,15 @@ ostree_sysroot_get_subbootversion (OstreeSysroot *self) * ostree_sysroot_get_booted_deployment: * @self: Sysroot * + * This function may only be called if the sysroot is loaded. + * * Returns: (transfer none) (nullable): The currently booted deployment, or %NULL if none */ OstreeDeployment * ostree_sysroot_get_booted_deployment (OstreeSysroot *self) { - g_return_val_if_fail (self->loadstate == OSTREE_SYSROOT_LOAD_STATE_LOADED, NULL); + g_assert (self); + g_assert (self->loadstate == OSTREE_SYSROOT_LOAD_STATE_LOADED); return self->booted_deployment; } @@ -1390,7 +1393,8 @@ ostree_sysroot_get_repo (OstreeSysroot *self, OstreeRepo * ostree_sysroot_repo (OstreeSysroot *self) { - g_return_val_if_fail (self->loadstate >= OSTREE_SYSROOT_LOAD_STATE_LOADED, NULL); + g_assert (self); + g_assert (self->loadstate >= OSTREE_SYSROOT_LOAD_STATE_LOADED); g_assert (self->repo); return self->repo; } From 3159e04980e567a918e532e168dea89112922460 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 4/7] fetcher/soup: Fix gcc `-fanalyzer` warning In general, we're probably going to need to change most of our `g_return_if_fail` to `g_assert`. The analyzer flags that the function can return `NULL`, but the caller isn't prepared for this. In practice, let's abort. --- src/libostree/ostree-fetcher-soup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c index 5f707175..7df48482 100644 --- a/src/libostree/ostree-fetcher-soup.c +++ b/src/libostree/ostree-fetcher-soup.c @@ -183,7 +183,7 @@ static OstreeFetcherPendingURI * pending_uri_ref (OstreeFetcherPendingURI *pending) { gint refcount; - g_return_val_if_fail (pending != NULL, NULL); + g_assert (pending); refcount = g_atomic_int_add (&pending->ref_count, 1); g_assert (refcount > 0); return pending; From f355482e1f82ee26ba0f202ff6c05b1b14ddc858 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 09:07:41 -0400 Subject: [PATCH 5/7] static-delta: Fix probably not actually possible NULL deref Flagged by `gcc -fanalyzer`. I didn't study this really deeply but I think it's not actually reachable. Anyways, let's catch it on general principle. --- src/ostree/ot-builtin-static-delta.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ostree/ot-builtin-static-delta.c b/src/ostree/ot-builtin-static-delta.c index ff31b574..0ce3cefb 100644 --- a/src/ostree/ot-builtin-static-delta.c +++ b/src/ostree/ot-builtin-static-delta.c @@ -669,7 +669,7 @@ ostree_builtin_static_delta (int argc, char **argv, OstreeCommandInvocation *inv return TRUE; /* Note early return */ } - if (!command->fn) + if (!command || !command->fn) { static_delta_usage (argv, TRUE); return glnx_throw (error, "Unknown \"static-delta\" subcommand '%s'", cmdname); From 54bf42c3e5f6603031f55f5e2187adb3c4f5c5da Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 09:10:59 -0400 Subject: [PATCH 6/7] utils: Fix unreachable `NULL` deref by adding assertion Again this one is just in theory, but let's add an assertion. --- src/libotutil/ot-gio-utils.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libotutil/ot-gio-utils.c b/src/libotutil/ot-gio-utils.c index f09ee8af..9ee6f7d5 100644 --- a/src/libotutil/ot-gio-utils.c +++ b/src/libotutil/ot-gio-utils.c @@ -82,10 +82,13 @@ ot_gfile_ensure_unlinked (GFile *path, GCancellable *cancellable, GError **error) { - if (unlink (gs_file_get_path_cached (path)) != 0) + g_assert (path); + const char *pathc = gs_file_get_path_cached (path); + g_assert (pathc); + if (unlink (pathc) != 0) { if (errno != ENOENT) - return glnx_throw_errno_prefix (error, "unlink(%s)", gs_file_get_path_cached (path)); + return glnx_throw_errno_prefix (error, "unlink(%s)", pathc); } return TRUE; } From 029a9d56c3bc2aaf7eb5fbaf763ae41c781c490b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 7/7] variantutil: Fix gcc `-fanalyzer` warnin Add some not-NULL assertions for return values from glib, and upgrade some `g_return_if_fail` to `g_assert`. --- src/libotutil/ot-variant-builder.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libotutil/ot-variant-builder.c b/src/libotutil/ot-variant-builder.c index 6636068e..e4347f39 100644 --- a/src/libotutil/ot-variant-builder.c +++ b/src/libotutil/ot-variant-builder.c @@ -909,6 +909,7 @@ ot_variant_builder_pre_add (OtVariantBuilderInfo *info, const GVariantMemberInfo *member_info; member_info = g_variant_type_info_member_info (info->type_info, info->n_children); + g_assert (member_info); alignment = member_info->type_info->alignment; } else if (g_variant_type_is_array (info->type)) @@ -959,6 +960,7 @@ ot_variant_builder_post_add (OtVariantBuilderInfo *info, const GVariantMemberInfo *member_info; member_info = g_variant_type_info_member_info (info->type_info, info->n_children); + g_assert (member_info); if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_OFFSET) ot_variant_builder_add_child_end (info); } @@ -1085,16 +1087,13 @@ ot_variant_builder_open (OtVariantBuilder *builder, OtVariantBuilderInfo *info = builder->head; OtVariantBuilderInfo *new_info; - g_return_val_if_fail (info->n_children < info->max_items, - FALSE); - g_return_val_if_fail (!info->expected_type || + g_assert (info->n_children < info->max_items); + g_assert (!info->expected_type || g_variant_type_is_subtype_of (type, - info->expected_type), - FALSE); - g_return_val_if_fail (!info->prev_item_type || + info->expected_type)); + g_assert (!info->prev_item_type || g_variant_type_is_subtype_of (info->prev_item_type, - type), - FALSE); + type)); if (!ot_variant_builder_pre_add (info, type, error)) return FALSE;