klibc - Jan 2019 - [klibc:update-dash] [BUILTIN] Use PRIdMAX instead of %j in printf

Commit-ID:  dd94c677c8f33bfbcce12b3c597bf558d60a7338
Gitweb:    
http://git.kernel.org/?p=libs/klibc/klibc.git;a=commit;h=dd94c677c8f33bfbcce12b3c597bf558d60a7338
Author:     Harald van Dijk <harald at gigawatt.nl>
AuthorDate: Fri, 23 Aug 2013 20:30:28 +1000
Committer:  Ben Hutchings <ben at decadent.org.uk>
CommitDate: Fri, 25 Jan 2019 02:57:21 +0000

[klibc] [BUILTIN] Use PRIdMAX instead of %j in printf

On 12/03/2012 05:59 PM, Harald van Dijk wrote:
> On 12/03/2012 08:42 AM, Roy wrote:
>> MSYS libc does not support %j[dXx] format, only %ll[dXx] is supported.
>>
>> diff --git a/src/bltin/printf.c b/src/bltin/printf.c
>> index 893295c..12ce660 100644
>> --- a/src/bltin/printf.c
>> +++ b/src/bltin/printf.c
>> @@ -319,11 +319,12 @@ mklong(const char *str, const char *ch)
>>         char *copy;
>>         size_t len;
>>
>> -       len = ch - str + 3;
>> +       len = ch - str + 4;
>>         STARTSTACKSTR(copy);
>>         copy = makestrspace(len, copy);
>> -       memcpy(copy, str, len - 3);
>> -       copy[len - 3] = 'j';
>> +       memcpy(copy, str, len - 4);
>> +       copy[len - 4] = 'l';
>> +       copy[len - 3] = 'l';
>>         copy[len - 2] = *ch;
>>         copy[len - 1] = '\0';
>>         return (copy);
>
> The calling code uses the result to print intmax_t and uintmax_t values.
> Printing intmax_t values with %lld is wrong, this will only work if
> intmax_t is really a typedef for long long (which may be true on your
> system, but is not required by the standard).
>
> The other patch that Jonathan linked to should work just fine.
Here's a slightly tweaked version of that patch. Regardless of whether
PRIdMAX is defined as "jd" or as "lld", the use of memcpy
here, first
copying "jd"/"lld" and the null byte, and only changing the
'd' after
that, surprisingly results in slightly shorter object code than the
original byte-by-byte approach, even though memcpy is fully inlined.
Perhaps that could be a reason for applying this, even if the original
reason for it, making the code work on not-quite-conforming systems,
isn't good enough to get it in dash.

Tested with normal glibc, and with glibc hacked to not provide PRIdMAX.

Reviewed-by: Jonathan Nieder <jrnieder at gmail.com>
Signed-off-by: Herbert Xu <herbert at gondor.apana.org.au>
Signed-off-by: Ben Hutchings <ben at decadent.org.uk>

---
 usr/dash/bltin/printf.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/usr/dash/bltin/printf.c b/usr/dash/bltin/printf.c
index dcff0992..add89095 100644
--- a/usr/dash/bltin/printf.c
+++ b/usr/dash/bltin/printf.c
@@ -320,16 +320,24 @@ out:
 static char *
 mklong(const char *str, const char *ch)
 {
+	/*
+	 * Replace a string like "%92.3u" with "%92.3"PRIuMAX.
+	 *
+	 * Although C99 does not guarantee it, we assume PRIiMAX,
+	 * PRIoMAX, PRIuMAX, PRIxMAX, and PRIXMAX are all the same
+	 * as PRIdMAX with the final 'd' replaced by the corresponding
+	 * character.
+	 */
+
 	char *copy;
 	size_t len;
 
-	len = ch - str + 3;
+	len = ch - str + sizeof(PRIdMAX);
 	STARTSTACKSTR(copy);
 	copy = makestrspace(len, copy);
-	memcpy(copy, str, len - 3);
-	copy[len - 3] = 'j';
+	memcpy(copy, str, len - sizeof(PRIdMAX));
+	memcpy(copy + len - sizeof(PRIdMAX), PRIdMAX, sizeof(PRIdMAX));
 	copy[len - 2] = *ch;
-	copy[len - 1] = '\0';
 	return (copy);
 }