[PATCH 2/2] lib/igt_facts : Cleanup

[Kamil Konieczny]

Hi Peter,
On 2025-01-21 at 09:32:32 +0100, Peter Senna Tschudin wrote:
> 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 at 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.

Good catch.

> 
> 
> > +	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.

Also good catch, thanks!

Regards,
Kamil

> 
> 
> > +	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);
>