Re: [EXTERNAL] Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects

["Xu, Baojun" <baojun.xu@xxxxxx>]

> ________________________________________
> From: Takashi Iwai <tiwai@xxxxxxx>
> Sent: 12 January 2025 16:26
> To: Xu, Baojun
> Cc: tiwai@xxxxxxx; robh+dt@xxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; lgirdwood@xxxxxxxxx; perex@xxxxxxxx; Ding, Shenghao; Navada Kanyana, Mukund; 13916275206@xxxxxxx; Hampiholi, Vallabha; P O, Vijeth; linux-sound@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; liam.r.girdwood@xxxxxxxxx; yung-chuan.liao@xxxxxxxxxxxxxxx; broonie@xxxxxxxxxx; antheas.dk@xxxxxxxxx; stuart.a.hayhurst@xxxxxxxxx; dan.carpenter@xxxxxxxxxx
> Subject: [EXTERNAL] Re: [PATCH v1] ALSA: hda/tas2781: Ignore SUBSYS_ID not found for tas2563 projects
> 
> On Sat, 11 Jan 2025 10: 57: 28 +0100, Baojun Xu wrote: > > Driver will return error if no SUBSYS_ID found in BIOS(acpi). > It will cause error in tas2563 projects, which have no SUBSYS_ID. > Change strncmp to strcmp to avoid warning
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source of this email and know the content is safe.
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK!vxdrHfwF_qfzogYE_xtoX7lTozH70XOKA0HRu-ioBvUvCv8QNkmAsR1WurvR9Zlh7GmU7A$>
> Report Suspicious
> 
> ZjQcmQRYFpfptBannerEnd
> 
> On Sat, 11 Jan 2025 10:57:28 +0100,
> Baojun Xu wrote:
> >
> > Driver will return error if no SUBSYS_ID found in BIOS(acpi).
> > It will cause error in tas2563 projects, which have no SUBSYS_ID.
> > Change strncmp to strcmp to avoid warning for weird length.
> 
> I don't understand the logic.
> The use of strncmp() there already matches only with the exact string
> "INT8866", not the substring, because you use sizeof("INT8866") which
> is 8 including the NUL-terminator.
> The only merit of strncmp() in this case is that it can be used for an
> unterminated char array.
> 
> In which situation do you see the problem and how does your patch
> improve / fix it?  Please give a more concrete example.
> 

Because it will have Smatch Static Checker warning:

	sound/pci/hda/tas2781_hda_i2c.c:146 tas2781_read_acpi()
	warn: strncmp() with weird length: 7 vs 8

sound/pci/hda/tas2781_hda_i2c.c
    144         if (IS_ERR(sub)) {
    145                 /* No subsys id in older tas2563 projects. */
--> 146                 if (!strncmp(hid, "INT8866", sizeof("INT8866")))

This patch will avoid this warning, and this hid will be a null terminated
"INT8866", so have no different when this code was go through.

> 
> thanks,
> 
> Takashi
> 
> >
> > Signed-off-by: Baojun Xu <baojun.xu@xxxxxx>
> > ---
> >  sound/pci/hda/tas2781_hda_i2c.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
> > index 0e42b87dadb8..61aec92f6536 100644
> > --- a/sound/pci/hda/tas2781_hda_i2c.c
> > +++ b/sound/pci/hda/tas2781_hda_i2c.c
> > @@ -143,7 +143,7 @@ static int tas2781_read_acpi(struct tasdevice_priv *p, const char *hid)
> >       sub = acpi_get_subsystem_id(ACPI_HANDLE(physdev));
> >       if (IS_ERR(sub)) {
> >               /* No subsys id in older tas2563 projects. */
> > -             if (!strncmp(hid, "INT8866", sizeof("INT8866")))
> > +             if (!strcmp(hid, "INT8866"))
> >                       goto end_2563;
> >               dev_err(p->dev, "Failed to get SUBSYS ID.\n");
> >               ret = PTR_ERR(sub);
> > --
> > 2.43.0
> >
> >
> 
>