* [PATCH] printf: use %ll instead of %j
@ 2012-12-03 7:42 Roy
2012-12-03 14:32 ` Jonathan Nieder
[not found] ` <50BCDA80.8000705@gigawatt.nl>
0 siblings, 2 replies; 5+ messages in thread
From: Roy @ 2012-12-03 7:42 UTC (permalink / raw)
To: dash
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);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] printf: use %ll instead of %j
2012-12-03 7:42 [PATCH] printf: use %ll instead of %j Roy
@ 2012-12-03 14:32 ` Jonathan Nieder
[not found] ` <50BCDA80.8000705@gigawatt.nl>
1 sibling, 0 replies; 5+ messages in thread
From: Jonathan Nieder @ 2012-12-03 14:32 UTC (permalink / raw)
To: Roy; +Cc: dash, Brian Koropoff
Hi Roy,
Roy wrote:
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] printf: use %ll instead of %j
[not found] ` <50BCDA80.8000705@gigawatt.nl>
@ 2012-12-03 17:58 ` Harald van Dijk
2012-12-04 6:38 ` Jonathan Nieder
0 siblings, 1 reply; 5+ messages in thread
From: Harald van Dijk @ 2012-12-03 17:58 UTC (permalink / raw)
To: Roy; +Cc: dash
[-- Attachment #1: Type: text/plain, Size: 1773 bytes --]
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.
Cheers,
Harald
[-- Attachment #2: printf-PRIdMAX.patch --]
[-- Type: text/x-patch, Size: 875 bytes --]
diff --git a/src/bltin/printf.c b/src/bltin/printf.c
index 893295c..5f9e81c 100644
--- a/src/bltin/printf.c
+++ b/src/bltin/printf.c
@@ -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);
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] printf: use %ll instead of %j
2012-12-03 17:58 ` Harald van Dijk
@ 2012-12-04 6:38 ` Jonathan Nieder
2013-08-23 10:30 ` Herbert Xu
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Nieder @ 2012-12-04 6:38 UTC (permalink / raw)
To: Harald van Dijk; +Cc: Roy, dash, Brian Koropoff
Harald van Dijk wrote:
> 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 <jrnieder@gmail.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] printf: use %ll instead of %j
2012-12-04 6:38 ` Jonathan Nieder
@ 2013-08-23 10:30 ` Herbert Xu
0 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2013-08-23 10:30 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Harald van Dijk, Roy, dash, Brian Koropoff
On Tue, Dec 04, 2012 at 06:38:24AM +0000, Jonathan Nieder wrote:
> Harald van Dijk wrote:
>
> > 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 <jrnieder@gmail.com>
Patch applied. Thanks!
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-08-23 10:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-03 7:42 [PATCH] printf: use %ll instead of %j Roy
2012-12-03 14:32 ` Jonathan Nieder
[not found] ` <50BCDA80.8000705@gigawatt.nl>
2012-12-03 17:58 ` Harald van Dijk
2012-12-04 6:38 ` Jonathan Nieder
2013-08-23 10:30 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).