#457 Fix printf %p output on big-endian platforms

[Basil Hussain]

Attached is a patch to fix %p format specifier output on big-endian platforms.

Previously, big-endian pointer values would be output with the bytes in the wrong order, as the code in device/lib/printf_large.c _print_format() for %p assumed the pointer value was little-endian.

const void *p = (void *)0xAABB;
printf("0x%04x %p", (uintptr_t)p, p);

Example STM8 and HC08 output before: 0xaabb 0xbbaa. Output after: 0xaabb 0xaabb.

See feature request #756.

[Philipp Klaus Krause]

Thanks. The patch looks good to me, but since is technically an incompatible change (which implies that the patch should bump the patch level in .version, and there should be an item added to the list in manual section 1.5), I'd like to first resolve all open questions, and then implement a solution.

I definitely do like that printing the uintptr_t gives the same output as %p.

I'd also like to see a regression test for this. It could print a pointer to a string via %p, then the corresponding uintptr_t via PRIxPTR, and use strcmp to compare the results.

[Basil Hussain]

You mean "incompatible" as in "changes a program's output that people may be reliant upon"? Yes, I agree, I guess it is that kind of change, even if the previous output was incorrect.

I can try and create a regression test. Would it be best to add it to support/regression/tests/snprintf.c.in, perhaps under the INT cases?

[Basil Hussain]

Upon further review, the snprintf regression test may not be able to do exactly what you propose. As-is, it only supports comparing a formatted string to an expected fixed string. Also, we don't have inttypes.h in the standard library, so we can't use PRIxPTR.

Do you think an acceptable test would be to do the basic comparison against a fixed string?

By the way, I noticed one further thing with regard to pointer formatting. The printf_large.c implementation permits - either by circumstance or intentionally - use of an uppercase %P specifier, which produces uppercase hex digits. This appears to be outside the C specification (the specifier that is, not its output). Do we want to take this opportunity to do something about this? Should be trivial - just force lower_case = 1 for the 'P' handler. But, again, this is an incompatible change for the sake of enforcing standard-compliance.

[Philipp Klaus Krause]

The standard doesn't mention %P. As an upper-case letter it is (or was - this might have changed in the latest standard) reserved for implementation extensions. But if there are no users, we could drop it to make the function a little bit smaller.
If I find the time, I'll add a basic inttypes.h with PRIxPTR before 4.3.0.

[Benedikt Freisen]

If I find the time, I'll add a basic inttypes.h with PRIxPTR before 4.3.0.

Or we could use mine, i.e. the one from [feature-requests:#597] that I intend to tidy up to finally get it into a release.

[Philipp Klaus Krause]

Yes, that looks like a better idea.

[Basil Hussain]

Here is a revised patch.

I have added a regression test for %p pointer output to the existing 'snprintf' test. It also covers the non-standard output for the ds390 and mcs51 platforms, as well as non-standard uppercase %P usage (but not for host platform).

Also bumped the version number as suggested and added a line item to section 1.5 of the manual describing the change to %p output.

[Philipp Klaus Krause]

Thanks. This is an obvious improvement over the current situation. In the long term, we should harmonize printing of pointers vs (u)intptr_t further, but I'll open a separate feature request for that: [feature-requests:#869].

[Philipp Klaus Krause]

Applied in [r13851], with changes in the manual.