Re: [PATCH v2] acpi: Permit OEM ID and OEM table ID fields to be changed

[Igor Mammedov]

On Wed, 23 Dec 2020 23:56:30 +0200
Marian Posteuca <posteuca@mutex.one> wrote:

> Thanks for the thorough review.
> 
> Igor Mammedov <imammedo@redhat.com> writes:
> > On Tue, 22 Dec 2020 13:33:53 +0200
> > Marian Posteuca <posteuca@mutex.one> wrote:
> >
> > I see defaults are now initialized in pcmc->oem_[table_]id fields,
> > and sometimes used from there, so question is why
> > do we need use_sig_oem and keeping old code
> >
> >     if (oem_id) {                                                           
> >      
> >         strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);               
> >      
> >     } else {                                                                
> >      
> >         memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);                          
> >      
> >     }                                                                       
> >      
> >                                                                             
> >      
> >     if ()) {                                                          
> >         strncpy((char *)h->oem_table_id, oem_table_id, 
> > sizeof(h->oem_table_id)); 
> >     } else {                                                                
> >      
> >         memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);                    
> >      
> >         memcpy(h->oem_table_id + 4, sig, 4);                                
> >      
> >     }  
> > I'd rather drop 'else' branches altogether and simplify to something like 
> > this
> >
> > g_assert(oem_id);
> > strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
> > g_assert(oem_table_id)
> > strncpy((char *)h->oem_table_id, oem_table_id, sizeof(h->oem_table_id));
> > + padding
> >
> > and make sure ids are properly propagated everywhere.
> >  
> 
> I'm not sure if I understood this point correctly. You want to remove the 
> appending
> of the sig part to the oem_table_id field, and just use whatever is
> passed by the caller for oem_table_id?
yes, according to spec unique oem_table_id helps only in distinguishing 
different
pieces of DSDT/SSDT tables, for other tables it doesn't make any sense to make 
it unique.
and this matches what real machines do.