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
