Skip to contentSkip to conversation

printf: use %ll instead of %j

PATCH In review Roy 4 participants 5 messages Dec 3, 2012 – Aug 23, 2013 1 message not in the archive

lore
Share & tools
lore

Roy

author
#0 lore raw

PATCH 1/1 printf: use %ll instead of %j

MSYS libc does not support %j[dXx] format, only %ll[dXx] is supported.

src/bltin/printf.c +4 -3
index 893295c..12ce660 100644
@@ -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);
#1 lore raw

Hi Roy,

Quoted 1 line · Roy

MSYS libc does not support %j[dXx] format, only %ll[dXx] is supported.

Are you sure that using the 'll' modifier here is more portable than 'j'?

FWIW there was a patch posted to use PRIdMAX before, which would be a more conservative thing to do and I think should work well.

http://thread.gmane.org/gmane.comp.shells.dash/564/focus=605

Hope that helps,
Jonathan

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw
Quoted 29 lines · Harald van Dijk
Quoted 12 lines · Roy

MSYS libc does not support %j[dXx] format, only %ll[dXx] is supported.

src/bltin/printf.c +1 -1
index 893295c..12ce660 100644
@@ -319,11 +319,12 @@ mklong(const char *str, const char *ch)
        char *copy;
        size_t len;
 
-       len = ch - str + 3;
+       len = ch - str + 4;

src/bltin/printf.c mklong(const char *str, const char *ch) · line 330

Quoted 10 lines
src/bltin/printf.c +3 -2
        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).

Quoted 1 line

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.

Cheers,
Harald

src/bltin/printf.c +12 -4
index 893295c..5f9e81c 100644
@@ -316,16 +316,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);	
 }
#5 lore raw
Quoted 8 lines · Jonathan Nieder
Quoted 1 line · Harald van Dijk

Here's a slightly tweaked version of that patch.

This is easier on the eyes than the version I sent, anyway. :)

For what it's worth,

Reviewed-by Jonathan Nieder <[email protected]>

Patch applied. Thanks!

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt