[PATCH RESEND] staging: nvec: use strcmp() instead of strncmp() with magic length

[PATCH RESEND] staging: nvec: use strcmp() instead of strncmp() with magic length

[Artem Lytkin]

Replace strncmp() with a hardcoded length of 30 with strcmp().
The bat_type string is already null-terminated (set two lines above),
so strncmp() with an arbitrary length is misleading and functionally
equivalent to strcmp().

Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
 drivers/staging/nvec/nvec_power.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/nvec/nvec_power.c b/drivers/staging/nvec/nvec_power.c
index 2faab9fdedef7..89dd997aaf36b 100644
--- a/drivers/staging/nvec/nvec_power.c
+++ b/drivers/staging/nvec/nvec_power.c
@@ -207,7 +207,7 @@ static int nvec_power_bat_notifier(struct notifier_block *nb,
 		 * This differs a little from the spec fill in more if you find
 		 * some.
 		 */
-		if (!strncmp(power->bat_type, "Li", 30))
+		if (!strcmp(power->bat_type, "Li"))
 			power->bat_type_enum = POWER_SUPPLY_TECHNOLOGY_LION;
 		else
 			power->bat_type_enum = POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
-- 
2.43.0

Re: [PATCH RESEND] staging: nvec: use strcmp() instead of strncmp() with magic length

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

On Thu, Jul 23, 2026 at 07:18:23PM +0300, Artem Lytkin wrote:
> Replace strncmp() with a hardcoded length of 30 with strcmp().
> The bat_type string is already null-terminated (set two lines above),
> so strncmp() with an arbitrary length is misleading and functionally
> equivalent to strcmp().
> 
> Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
> ---
>  drivers/staging/nvec/nvec_power.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I don't really see a point in this. strncmp() doesn't do any harm and it
isn't misleading either. It's just very explicit that it should never
check more than those 30 characters that can fit into the type string.

Technically res->length could be larger than 30, in which case the
memcpy() might copy more than those 30. Judging by the nvec core code we
never sanity check the length, so even that memcpy() could be entirely
wrong.

Either way, this change by itself doesn't look useful.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH RESEND] staging: nvec: use strcmp() instead of strncmp() with magic length

[Greg KH]

On Thu, Jul 23, 2026 at 07:18:23PM +0300, Artem Lytkin wrote:
> Replace strncmp() with a hardcoded length of 30 with strcmp().

I thought we were trying to get rid of strcmp() usage?  Why add it
back?

thanks,

greg k-h

Re: [PATCH RESEND] staging: nvec: use strcmp() instead of strncmp() with magic length

[Dan Carpenter]

On Tue, Jul 28, 2026 at 09:09:23AM +0200, Greg KH wrote:
> On Thu, Jul 23, 2026 at 07:18:23PM +0300, Artem Lytkin wrote:
> > Replace strncmp() with a hardcoded length of 30 with strcmp().
> 
> I thought we were trying to get rid of strcmp() usage?  Why add it
> back?

I don't think we're trying to get rid of strcmp().

strncmp() is for prefixes and and strcmp() is for whole words.
They're not the same.  strncmp() with a fixed string always
seemed like nonsense to me and it causes a static checker in
unpublished Smatch checks because we've seen that bug where
people wanted to check the prefix but used the wrong number of
characters.

regards,
dan carpenter

Re: [PATCH RESEND] staging: nvec: use strcmp() instead of strncmp() with magic length

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

On Tue, Jul 28, 2026 at 06:29:06PM +0300, Dan Carpenter wrote:
> On Tue, Jul 28, 2026 at 09:09:23AM +0200, Greg KH wrote:
> > On Thu, Jul 23, 2026 at 07:18:23PM +0300, Artem Lytkin wrote:
> > > Replace strncmp() with a hardcoded length of 30 with strcmp().
> > 
> > I thought we were trying to get rid of strcmp() usage?  Why add it
> > back?
> 
> I don't think we're trying to get rid of strcmp().
> 
> strncmp() is for prefixes and and strcmp() is for whole words.
> They're not the same.  strncmp() with a fixed string always
> seemed like nonsense to me and it causes a static checker in
> unpublished Smatch checks because we've seen that bug where
> people wanted to check the prefix but used the wrong number of
> characters.

Maybe the commit message should be updated. It makes the argument that
bat_type is NUL-terminated and that that is the reason why changing this
to strcmp() is safe. But it's not. There doesn't seem to be any sanity
checking on the EC data that serves as input to this, so the length of
the string that is copied could be any arbitrary value for all we know,
meaning we may already be writing past the end of bat_type and insert
the NUL some place invalid to begin with (we should probably fix that at
some point).

The real reason the conversion is safe is because strcmp() stops
searching the haystack if the needle is exhausted. Since the needle is
only 2 characters in this case, *that's* why we don't have to worry
about the limiting the search.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]