Re: [RFC 1/2] lib/vsprintf: Add support for pte_t

From: Petr Mladek
Date: Thu Jun 19 2025 - 10:02:26 EST


On Wed 2025-06-18 09:42:34, Anshuman Khandual wrote:
> Add a new format for printing page table entries.

How many users do you explect, please?

This patch adds only one caller. It does not justify the added complexity.

> @@ -2542,6 +2545,23 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
> default:
> return error_string(buf, end, "(einval)", spec);
> }
> + case 'p':

Please, do not opencode this in the already very long switch().
Move it to a helper function.


> + if (fmt[1] == 't' && fmt[2] == 'e') {
> + pte_t *pte = (pte_t *)ptr;

If the value (pointer) gets dereferenced then please add a basic
check:

if (check_pointer(&buf, end, ptr, spec))
return buf;

> + spec.field_width = 10;
> + spec.precision = 8;

Is she precision = 8 really needed?
I guess that .field_width + ZEROPAD would do the trick.

And them maybe special_hex_number() might be used instead of number()
and safe a lot of code.

> + spec.base = 16;
> + spec.flags = SPECIAL | SMALL | ZEROPAD;
> + if (sizeof(pte_t) == sizeof(u64)) {
> + u64 val = pte_val(*pte);
> +
> + return number(buf, end, val, spec);
> + }
> + WARN_ONCE(1, "Non standard pte_t\n");

This is nasty. It should be a compile-time check. And the code should
get fixed on all architectures. If it is not easy then
it might be a signal that the generic %ppte flag is not a good idea.

> + return error_string(buf, end, "(einval)", spec);
> + }
> + fallthrough;
> default:
> return default_pointer(buf, end, ptr, spec);
> }

Best Regards,
Petr