[PATCH dev-5.4] hwmon: peci: change label strings to reflect natural numbers

Jae Hyun Yoo jae.hyun.yoo at linux.intel.com
Thu Feb 27 09:19:01 AEDT 2020


On 2/26/2020 2:11 PM, Joel Stanley wrote:
> On Wed, 26 Feb 2020 at 22:07, Jae Hyun Yoo <jae.hyun.yoo at linux.intel.com> wrote:
>>
>> Hi Joel,
>>
>> On 2/26/2020 1:54 PM, Joel Stanley wrote:
>>> On Tue, 11 Feb 2020 at 23:47, Jae Hyun Yoo <jae.hyun.yoo at linux.intel.com> wrote:
>>>>
>>>> This commit changes label strings to reflect user friendly natural
>>>> numbers like 'Core 1' instead of 'Core 0' and 'DIMM A1' instead of
>>>> 'DIMM A0'.
>>>>
>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo at linux.intel.com>
>>>> ---
>>>>    drivers/hwmon/peci-cputemp.c  | 2 +-
>>>>    drivers/hwmon/peci-dimmtemp.c | 2 +-
>>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>>    mode change 100644 => 100755 drivers/hwmon/peci-cputemp.c
>>>>    mode change 100644 => 100755 drivers/hwmon/peci-dimmtemp.c
>>>
>>> I don't think this was intended.
>>>
>>> I fixed it up for you, but please try to figure out what went wrong so
>>> it doesn't happen in the future.
>>
>> It happened while upstreaming it. I changed the static label string
>> table as a dynamic table to address review comments, and I missed
>> this mismatch. My bad. It'll be fixed in the next upstreaming spin.
> 
> I was just referring to the files becoming executable :)

Yeah, I was aware that just after sending my previous email.

> I am surprised by the code that makes string allocation dynamic. As
> you know the number of strings (one per sensor), and you're allocating
> a fixed size for each string, I would suggest you're better off
> allocating all of the strings at once.
> 
> Regardless, I suggest you change the calls to snprintf, as if a
> machine has more than 999 DIMMs it will overflow the string :)

The machine wouldn't come in the near future but I agree with you. Let
me replace the sprintf with snprintf. Will submit a new patch.

Thanks! :)

Jae

> Cheers,
> 
> Joel
> 


More information about the openbmc mailing list