Messages in this thread | | | Subject | Re: [PATCH 1/2] [v2] lib/hexdump: introduce DUMP_PREFIX_UNHASHED for unhashed addresses | From | Timur Tabi <> | Date | Mon, 18 Jan 2021 09:57:55 -0600 |
| |
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.
| |