lkml.org 
[lkml]   [2021]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] [v2] lib/hexdump: introduce DUMP_PREFIX_UNHASHED for unhashed addresses
From
Date
On 1/18/21 4:03 AM, Andy Shevchenko wrote:
> On Sun, Jan 17, 2021 at 12:12 AM Timur Tabi <timur@kernel.org> wrote:
>
> (Hint: -v<n> to the git format-patch will create a versioned subject
> prefix for you automatically)

I like to keep the version in the git repo itself so that I don't need
to keep track of it separately, but thanks for the hint. I might use it
somewhere else.

>> Hashed addresses are useless in hexdumps unless you're comparing
>> with other hashed addresses, which is unlikely. However, there's
>> no need to break existing code, so introduce a new prefix type
>> that prints unhashed addresses.
>
> Any user of this? (For the record, I don't see any other mail except this one)

It's patch #2 of this set. They were all sent together.

http://lkml.iu.edu/hypermail/linux/kernel/2101.2/00245.html

Let me know what you think.

>> DUMP_PREFIX_NONE,
>> DUMP_PREFIX_ADDRESS,
>> - DUMP_PREFIX_OFFSET
>> + DUMP_PREFIX_OFFSET,
>> + DUMP_PREFIX_UNHASHED,
>
> Since it's an address, I would like to group them together, i.e. put
> after DUMP_PREFIX_ADDRESS.

I didn't want to change the numbering of any existing enums, just in
case there are users that accidentally hard-code the values. I'm trying
to make this patch as unobtrusive as possible.

> Perhaps even add _ADDRESS to DUMP_PREFIX_UNHASHED, but this maybe too
long.

I think DUMP_PREFIX_ADDRESS_UNHASHED is too long.

>> + * @prefix_type: controls whether prefix of an offset, hashed address,
>> + * unhashed address, or none is printed (%DUMP_PREFIX_OFFSET,
>> + * %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_UNHASHED, %DUMP_PREFIX_NONE)
>
> Yeah, exactly, here you use different ordering.

That's because it's a comment.

>> + * @prefix_type: controls whether prefix of an offset, hashed address,
>> + * unhashed address, or none is printed (%DUMP_PREFIX_OFFSET,
>> + * %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_UNHASHED, %DUMP_PREFIX_NONE)
>
> In both cases I would rather use colon and list one per line. What do you think?

Hmmmm.... if I'm going to change the patch anyway, sure.

>> + case DUMP_PREFIX_UNHASHED:
>
> Here is a third type of ordering, can you please be consistent?
>
>> case DUMP_PREFIX_ADDRESS:

Fair enough.

\
 
 \ /
  Last update: 2021-01-18 17:18    [W:0.142 / U:1.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site