[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v2 02/10] Drop unused static function return values
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [RFC v2 02/10] Drop unused static function return values |
Date: |
Wed, 3 Aug 2022 11:46:26 +0100 |
User-agent: |
Mutt/2.2.6 (2022-06-05) |
* Alberto Faria (afaria@redhat.com) wrote:
> Make non-void static functions whose return values are ignored by
> all callers return void instead.
>
> These functions were found by static-analyzer.py.
>
> Not all occurrences of this problem were fixed.
>
> Signed-off-by: Alberto Faria <afaria@redhat.com>
<snip>
> diff --git a/migration/migration.c b/migration/migration.c
> index e03f698a3c..4698080f96 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -175,7 +175,7 @@ static MigrationIncomingState *current_incoming;
>
> static GSList *migration_blockers;
>
> -static bool migration_object_check(MigrationState *ms, Error **errp);
> +static void migration_object_check(MigrationState *ms, Error **errp);
> static int migration_maybe_pause(MigrationState *s,
> int *current_active_state,
> int new_state);
> @@ -4485,15 +4485,15 @@ static void migration_instance_init(Object *obj)
> * Return true if check pass, false otherwise. Error will be put
> * inside errp if provided.
> */
> -static bool migration_object_check(MigrationState *ms, Error **errp)
> +static void migration_object_check(MigrationState *ms, Error **errp)
> {
I'm not sure if this is a good change.
Where we have a function that returns an error via an Error ** it's
normal practice for us to return a bool to say whether it generated an
error.
Now, in our case we only call it with error_fatal:
migration_object_check(current_migration, &error_fatal);
so the bool isn't used/checked.
So I'm a bit conflicted:
a) Using error_fatal is the easiest way to handle this function
b) Things taking Error ** normally do return a flag value
c) But it's not used in this case.
Hmm.
Dave
> MigrationCapabilityStatusList *head = NULL;
> /* Assuming all off */
> - bool cap_list[MIGRATION_CAPABILITY__MAX] = { 0 }, ret;
> + bool cap_list[MIGRATION_CAPABILITY__MAX] = { 0 };
> int i;
>
> if (!migrate_params_check(&ms->parameters, errp)) {
> - return false;
> + return;
> }
>
> for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
> @@ -4502,12 +4502,10 @@ static bool migration_object_check(MigrationState
> *ms, Error **errp)
> }
> }
>
> - ret = migrate_caps_check(cap_list, head, errp);
> + migrate_caps_check(cap_list, head, errp);
>
> /* It works with head == NULL */
> qapi_free_MigrationCapabilityStatusList(head);
> -
> - return ret;
> }
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
- Re: [RFC v2 02/10] Drop unused static function return values,
Dr. David Alan Gilbert <=
Re: [RFC v2 02/10] Drop unused static function return values, Daniel P . Berrangé, 2022/08/03