Re: [PATCH v2] hwmon: (tps53679) Fix device ID comparison and printing in tps53676_identify()

[Andy Shevchenko]

On Mon, Mar 30, 2026 at 03:56:40PM +0000, Pradhan, Sanman wrote:
> 
> tps53676_identify() uses strncmp() to compare the device ID buffer
> against a byte sequence containing embedded non-printable bytes
> (\x53\x67\x60). strncmp() is semantically wrong for binary data
> comparison; use memcmp() instead.
> 
> Additionally, the buffer from i2c_smbus_read_block_data() is not
> NUL-terminated, so printing it with "%s" in the error path is
> undefined behavior and may read past the buffer. Use "%*ph" to
> hex-dump the actual bytes returned.
> 
> Per the datasheet, the expected device ID is the 6-byte sequence
> 54 49 53 67 60 00 ("TI\x53\x67\x60\x00"), so compare all 6 bytes
> including the trailing NUL.

Your patch seems okay to me. But just for your information I would do it
differently. Id est the sequence you got for the comparison is the perfect
ASCII one, but this is a coincident as the actual ID is in FourCC BCD.

The better check is to compare first two bytes to TI and device ID separately
as FourCC. Do you want me to send a patch or you can do it yourself?

(Also note that there is no need to use *ph as we know the length and it's
 always the same, so %6ph suffice, but again, you want to print the FourCC,
 %p4c.)

...

>  	ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
>  	if (ret < 0)
>  		return ret;
> -	if (strncmp("TI\x53\x67\x60", buf, 5)) {
> -		dev_err(&client->dev, "Unexpected device ID: %s\n", buf);
> +	if (ret != 6 || memcmp(buf, "TI\x53\x67\x60\x00", 6)) {
> +		dev_err(&client->dev, "Unexpected device ID: %*ph\n", ret, buf);
>  		return -ENODEV;
>  	}

#define TPS53676_DEVICE_ID_MAGIC	0x53676000

	if (ret != 6)
		dev_err(&client->dev, "Malformed response\n");
		return -EIO;
	}
	if (strncmp(buf, "TI", 2)) {
		dev_err(&client->dev, "Unexpected vendor ID: %2pE\n", buf); // use %pE since it maybe unprintable
		return -ENODEV;
	}
	if (get_unaligned_be32(buf + 2) != _DEVICE_ID_MAGIC) {
		dev_err(&client->dev, "Unexpected device ID: %p4c\n", buf + 2);
		return -ENODEV;
	}


-- 
With Best Regards,
Andy Shevchenko