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

[Petr Mladek <pmladek@suse.com>]

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