linux-kernel - Re: [PATCH] hwmon: (pmbus/acbel-fsg032) Add firmware version debugfs attribute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 29 Jun 2023 12:09:17 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Dan Carpenter <dan.carpenter@...aro.org>,
        oe-kbuild@...ts.linux.dev, Eddie James <eajames@...ux.ibm.com>,
        linux-hwmon@...r.kernel.org
Cc:     lkp@...el.com, oe-kbuild-all@...ts.linux.dev,
        linux-kernel@...r.kernel.org, jdelvare@...e.com,
        lakshmiy@...ibm.com
Subject: Re: [PATCH] hwmon: (pmbus/acbel-fsg032) Add firmware version debugfs
 attribute

On 6/29/23 11:59, Dan Carpenter wrote:
> On Thu, Jun 29, 2023 at 09:53:16AM +0300, Dan Carpenter wrote:
>> d2c6444389b625 Eddie James 2023-06-27  22  	char out[8];
>> d2c6444389b625 Eddie James 2023-06-27  23  	int rc;
>> d2c6444389b625 Eddie James 2023-06-27  24  	int i;
>> d2c6444389b625 Eddie James 2023-06-27  25
>> d2c6444389b625 Eddie James 2023-06-27  26  	rc = pmbus_lock_interruptible(client);
>> d2c6444389b625 Eddie James 2023-06-27  27  	if (rc)
>> d2c6444389b625 Eddie James 2023-06-27  28  		return rc;
>> d2c6444389b625 Eddie James 2023-06-27  29
>> d2c6444389b625 Eddie James 2023-06-27  30  	rc = i2c_smbus_read_block_data(client, ACBEL_MFR_FW_REVISION, data);
>> d2c6444389b625 Eddie James 2023-06-27  31  	pmbus_unlock(client);
>> d2c6444389b625 Eddie James 2023-06-27  32  	if (rc < 0)
>> d2c6444389b625 Eddie James 2023-06-27  33  		return rc;
>> d2c6444389b625 Eddie James 2023-06-27  34
>> d2c6444389b625 Eddie James 2023-06-27  35  	for (i = 0; i < rc && i < 3; ++i)
>> d2c6444389b625 Eddie James 2023-06-27 @36  		snprintf(&out[i * 2], 3, "%02X", data[i]);
>>
>> If data[i] is negative this will print FFFFFFF1 etc.  (This is an x86
>> config...  Did we ever merge that patch to make char signed by default?)
> 
> I meant unsigned not signed.  But actually we debated both ways...
> Signed by default would annoy PowerPC devs since they try to really
> lean into the fact that char is unsigned on that arch.  :P
> 
> https://lwn.net/Articles/911914/
> 

As if anything would be easy nowadays ;-). Anyway, in this case, the array
should be explicitly unsigned, so changing the type to u8 was the right
thing to do. Also, the driver should be usable on non-Intel systems,
which is another reason to make the type sign-specific (even more so in
the context of the above discussion).

Thanks,
Guenter