Re: [PATCH] bios-tables-test: do not ignore allowed diff list

[Michael S. Tsirkin]

On Mon, Oct 31, 2022 at 01:31:04PM +0100, Igor Mammedov wrote:
> On Mon, 31 Oct 2022 06:52:11 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Mon, Oct 31, 2022 at 11:49:42AM +0100, Igor Mammedov wrote:
> > > On Thu, 27 Oct 2022 11:11:48 -0400
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >   
> > > > we had such a beautiful structure for updating
> > > > expected files, designed to keep bisect working.
> > > > It turns out that we ignored the result of
> > > > the allow list checks unless all tables matched
> > > > anyway.
> > > > 
> > > > Sigh.  
> > > 
> > > strange,
> > > it seems to be working fine (I mean white-listing) here  
> > 
> > it's pretty clear no? if we only check test_acpi_find_diff_allowed
> > when all tables match anyway, it won't help test pass.
> 
> currently all_tables_match is accumulated value that starts with 'true'

Problem is, it can be false because of another unrelated table.







> and with the meaning 'do not explode unless at least a table was not
> explicitly whitelisted'



> [...]
> > > >  
> > > > -        all_tables_match = all_tables_match &&
>   '&&' here serves as a trigger that lets flip always initial 
> 'all_tables_match = true'
> 
> > > > +        all_tables_match = all_tables_match ||
>   once it changes to '||' the all_tables_match will never flip to false
> and trigger
>   g_assert(all_tables_match);
> at the end, when there is unexpected (non-whitelisted) table mismatch.
> 
> Am I missing something?


I agree this patch is bad.

I did not record the issue I saw and don't really want to go
debugging. Will drop for now.

> > > >              test_acpi_find_diff_allowed(exp_sdt);
> > > >  
> > > >          /*  
> >