Re: [Crash-utility] [PATCH] Fix kernel version macros for revision numbers over 255
On 2023/05/11 17:49, lijiang wrote:
On Thu, May 11, 2023 at 8:38 AM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com>
wrote:
> On 2023/05/10 18:03, lijiang wrote:
>> Hi, Kazu
>> Thank you for the patch.
>> On Wed, May 10, 2023 at 3:09 PM HAGIO KAZUHITO(萩尾 一仁) <
> k-hagio-ab(a)nec.com>
>> wrote:
>>
>>> From: Kazuhito Hagio <k-hagio-ab(a)nec.com>
>>>
>>> The current comparison macros for kernel version shift minor number only
>>> 8 bits. This can cause an unexpected result on kernels with revision
>>> number over 255, e.g. Linux 4.14.314.
>>>
>>>
>> For this case, I saw kernel deal with it as below:
>>
>> #define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + ((c) >
255 ?
>> 255 : (c)))
>>
>> Can you try to imitate the above macro definition and help to confirm if
> it
>> can also work for your case? If yes, it should be good to follow up
> kernel
>> change.
>
> um, if we need to check a revision number over 255, the KERNEL_VERSION
> macro will not work. For example,
>
>
Do you mean the following changes can not work?
#define THIS_KERNEL_VERSION ((kt->kernel_version[0] << 16) + \
(kt->kernel_version[1] << 8) + \
((kt->kernel_version[2] > 255) ? 255 :
(kt->kernel_version[2])))
#define LINUX(x,y,z) (((uint)(x) << 16) + ((uint)(y) << 8) + ((uint)(z)
>
255 ? 255 : (uint)(z)))
No, in the current crash, this will work. Because there is no
if-conditional that checks a revision value over 255.
if (THIS_KERNEL_VERSION < LINUX(4,14,300))
But yes, when we need to check like this, the above cannot work. For
example, THIS_KERNEL_VERSION on 4.14.310 equals to LINUX(4,14,300).
So, my thought is that it would be better to change the shift amounts
now for future changes with the small effort, rather than change it
again in the future.
And the version checks are closed in crash, we don't need to emulate
the kernel macros.
Thanks,
Kazu