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

[Takashi Iwai <tiwai@suse.de>]

On Mon, 13 Jan 2025 07:41:02 +0100,
Xu, Baojun wrote:
> 
> > ________________________________________
> > From: Takashi Iwai <tiwai@suse.de>
> > Sent: 12 January 2025 16:26
> > To: Xu, Baojun
> > Cc: tiwai@suse.de; robh+dt@kernel.org; andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com; perex@perex.cz; Ding, Shenghao; Navada Kanyana, Mukund; 13916275206@139.com; Hampiholi, Vallabha; P O, Vijeth; linux-sound@vger.kernel.org; linux-kernel@vger.kernel.org; liam.r.girdwood@intel.com; yung-chuan.liao@linux.intel.com; broonie@kernel.org; antheas.dk@gmail.com; stuart.a.hayhurst@gmail.com; dan.carpenter@linaro.org
> > 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.

Ah, so your change is only about the last line in the sentence.
The former two lines describe what the current driver already does.

As the driver behavior won't change, your current patch subject and
description are way too confusing.  Please rephrase them and
resubmit.  It's nothing but just a code refactoring that eventually
shuts up the Smatch warning, after all.


thanks,

Takashi