Re: [PATCH 2/2] lib/igt_facts : Cleanup

[Peter Senna Tschudin <peter.senna@linux.intel.com>]

Hi Pravalika,

Please wait for Kamil to answer my questions in the original
thread before changing the code.

How did you test this patch?

Please see my comments below.

Thanks

On 21.01.2025 07:32, Pravalika Gurram wrote:
> fix BOOL_COMPARISON, COMPARISON_TO_NULL,
> LINE_SPACING, PARENTHESIS_ALIGNMENT,
> Remove extern from header warning from checkpatch
> 
> 'Fixes: 1a9c3117328 ("lib/igt_facts: Library and unit testing")'
> 
> Signed-off-by: Pravalika Gurram <pravalika.gurram@intel.com>
> ---
>  lib/igt_facts.c | 20 ++++++++++----------
>  lib/igt_facts.h |  3 ++-
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/igt_facts.c b/lib/igt_facts.c
> index bd6742065..6e7efbaf3 100644
> --- a/lib/igt_facts.c
> +++ b/lib/igt_facts.c
> @@ -677,18 +677,18 @@ static void igt_facts_test_add_get(struct igt_list_head *head)
>  	const char *last_test = NULL;
>  
>  	ret = igt_facts_list_add(name, value, last_test, head);
> -	igt_assert(ret == true);
> +	igt_assert(ret);
>  
>  	/* Assert that there is one element in the linked list */
>  	igt_assert_eq(igt_list_length(head), 1);
>  
>  	/* Assert that the element in the linked list is the one we added */
>  	fact = igt_facts_list_get(name, head);
> -	igt_assert(fact != NULL);
> +	igt_assert(fact);
>  	igt_assert_eq(strcmp(fact->name, name), 0);
>  	igt_assert_eq(strcmp(fact->value, value), 0);
> -	igt_assert(fact->present == true);
> -	igt_assert(fact->last_test == NULL);
> +	igt_assert(fact->present);
> +	igt_assert(!fact->last_test);
>  }
>  
>  /**
> @@ -729,16 +729,16 @@ static void igt_facts_test_mark_and_sweep(struct igt_list_head *head)
>  
>  	/* Assert that the two updated elements are present */
>  	fact = igt_facts_list_get(name1, head);
> -	igt_assert(fact != NULL);
> -	igt_assert(fact->present == true);
> +	igt_assert(!fact);
This one is wrong.


> +	igt_assert(fact->present);
>  
>  	fact = igt_facts_list_get(name2, head);
> -	igt_assert(fact != NULL);
> -	igt_assert(fact->present == true);
> +	igt_assert(!fact);
This one is wrong too.


> +	igt_assert(fact->present);
>  
>  	/* Assert that the third element was deleted */
>  	fact = igt_facts_list_get(name3, head);
> -	igt_assert(fact == NULL);
> +	igt_assert(!fact);
>  }
>  
>  /**
> @@ -766,7 +766,7 @@ __noreturn void igt_facts_test(void)
>  	/* Assert that igt_facts_list_mark_and_sweep() cleans up the list */
>  	igt_assert(igt_list_empty(&igt_facts_list_pci_gpu_head) == false);
>  	igt_facts_list_mark_and_sweep(&igt_facts_list_pci_gpu_head);
> -	igt_assert(igt_list_empty(&igt_facts_list_pci_gpu_head) == true);
> +	igt_assert(igt_list_empty(&igt_facts_list_pci_gpu_head));
>  
>  	/* Test the mark and sweep pattern used to delete elements
>  	 * from the list
> diff --git a/lib/igt_facts.h b/lib/igt_facts.h
> index e96f88083..60d702977 100644
> --- a/lib/igt_facts.h
> +++ b/lib/igt_facts.h
> @@ -37,7 +37,8 @@ struct igt_facts_config {
>  	bool enabled;
>  	bool disable_udev;
>  };
> -extern struct igt_facts_config igt_facts_config;
> +
> +struct igt_facts_config igt_facts_config;
>  
>  void igt_facts_lists_init(void);
>  void igt_facts(const char *last_test);