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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <92a89dbb-7031-459a-ad5a-69b29b08cfc8@arm.com>
Date: Fri, 20 Jun 2025 12:23:47 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: linux-mm@...ck.org, Rasmus Villemoes <linux@...musvillemoes.dk>,
 Sergey Senozhatsky <senozhatsky@...omium.org>, Petr Mladek
 <pmladek@...e.com>, Steven Rostedt <rostedt@...dmis.org>,
 Jonathan Corbet <corbet@....net>, Andrew Morton <akpm@...ux-foundation.org>,
 David Hildenbrand <david@...hat.com>, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [RFC 1/2] lib/vsprintf: Add support for pte_t



On 19/06/25 5:30 PM, Andy Shevchenko wrote:
> On Thu, Jun 19, 2025 at 03:05:10PM +0530, Anshuman Khandual wrote:
>> On 18/06/25 11:16 PM, Andy Shevchenko wrote:
>>> On Wed, Jun 18, 2025 at 09:42:34AM +0530, Anshuman Khandual wrote:
>>>> Add a new format for printing page table entries.
>>>
>>>> Cc: Petr Mladek <pmladek@...e.com>
>>>> Cc: Steven Rostedt <rostedt@...dmis.org>
>>>> Cc: Jonathan Corbet <corbet@....net>
>>>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>>>> Cc: David Hildenbrand <david@...hat.com>
>>>> Cc: linux-doc@...r.kernel.org
>>>> Cc: linux-kernel@...r.kernel.org
>>>> Cc: linux-mm@...ck.org
>>>
>>> Please. move these to be after the '---' cutter line below. Just leave SoB tag
>>> alone. This will have the same effect w/o polluting commit message.
>>>
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
>>>> ---
>>>
>>> (somewhere here is a good place for all your Cc: tags)
>>
>> Is not it better to also capture the Cc: list in the commit message.
> 
> No it's worse. One may easily get the same from lore. Can you give a good
> justification for the polluting message with 8 lines over a single line of the
> useful information, please?

Will drop the Cc: list from the commit message and move it below '---'
cutter line as suggested earlier.

> 
>> Seems like such has been the practice for various patches on the MM
>> list. But not sure if that is an expected standard for all patches.
> 
> It's not an MM subsystem.
> 
> ...
> 
>>>> +Print standard page table entry pte_t.
>>>> +
>>>> +Passed by reference.
>>>> +
>>>> +Examples for a 64 bit page table entry, given &(u64)0xc0ffee::
>>>
>>> What does this mean?
>>
>> 64 bit address containing value the 0xc0ffee
> 
> Please, make it 64-bit address. The example as is is quite confusing.
Agreed it is some what confusing - will fix it.

> 
>>>> +        %ppte   0x00c0ffee
>>>
>>> Can it be ever 64-bit?
>> I am sorry - did not get that. pte_t contained value can be 64
>> bits if that's what you meant.
> 
> Yes, see above why I have such a question.

Got it.

> 
> ...
> 
>>>> +			spec.field_width = 10;
>>>> +			spec.precision = 8;
>>>> +			spec.base = 16;
>>>> +			spec.flags = SPECIAL | SMALL | ZEROPAD;
>>>
>>> Do not duplicate code we have already in the file.
>> I am sorry - did not get that. Is the above flag combination some
>> how wrong ?
> 
> It's dup. Please, take your time to find the very similar piece of code in one
> of the helper functions we have.

Are you referring to special_hex_number() ?

> 
> I recommend you to look at the history of the changes in this file for when the
> new specifier was added and how it is implemented> 
> ...
> 
>> Could you please kindly elaborate on the code duplication problem
>> you have mentioned earlier. I might not understand your concern
>> here correctly.
> 
> Just find the same or similar pieces of code elsewhere in the same file.
> Use them.
> Will go through previous print format additions and re-work the patches
accommodating various suggestions. Thanks for your review.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ