Re: [PATCH] vsprintf: sanely handle NULL passed to %pe

[Petr Mladek]

On Sat 2020-02-22 00:52:27, Rasmus Villemoes wrote:
> On 21/02/2020 14.05, Petr Mladek wrote:
> > On Thu 2020-02-20 16:02:48, Ilya Dryomov wrote:
> 
> >> I would like to see it in 5.6, so that it is backported to 5.4 and 5.5.
> > 
> Sorry to be that guy, but yes, I'm against changing the behavior of
> vsnprintf() without at least some test(s) added to the test suite - the
> lack of machine-checked documentation in the form of tests is what led
> to that regression in the first place.

I would not call this regression. It was intentional. The change in
5.2 unified the behavior for the other %p? modifiers. I simply did
not care about plain %p because it was already crippled by the hashing.

I am fine with the proposed change. But the more I think about it
the less I want to rush it in for 5.6. The proposed patch changes
the behavior again:

Value           Output v5.1       Output v5.2      Proposal

NULL                       (null) 00000000<.hash.> 0000000000000000
fffffffffffffffe 00000000<.hash.> 00000000<.hash.> fffffffffffffffe
ffffffff12345678 00000000<.hash.> 00000000<.hash.> 00000000<.hash.>

I do not want to change this in rc phase. I would really like to wait
for 5.7.


> But I agree that there's no point adding another helper function and
> muddying the test suite even more (especially as the name error_pointer
> is too close to the name errptr() I chose a few months back for the %pe).
> 
> So how about
> 
> - remove the now stale test_hashed("%p", NULL); from null_pointer()
> - add tests of "%p", NULL and "%p", ERR_PTR(-123) to plain()
> 
> and we save testing the "%px" case for when we figure out a good name
> for a helper for that (explicit_pointer? pointer_as_hex?)

In this, case I would prefer to fix the tests properly first. There
have been only few commits in lib/test_printf.c since 5.2. And they
should not conflict with the changes proposed at
https://lkml.kernel.org/r/20200220125707.hbcox3xgevpezq4l@xxxxxxxxxxxxxxx
So it should be easy to backport as well.

If you want, I could sent the cleanup patch properly for review.

Best Regards,
Petr