View Issue Details

IDProjectCategoryView StatusLast Update
0000528LDMud 3.3Portabilitypublic2018-01-29 21:57
Reporterzesstra Assigned Tozesstra  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Platformx86_64OSMacOS XOS Version10.5.x
Product Version3.3 
Fixed in Version3.3.718 
Summary0000528: sprintf() fails to print int values larger than 2^32 - 1 correctly
Descriptionsprintf() assumes that LPC ints (T_NUMBER) are 32-bit ints and wraps at 2^31 to negative values and then again after 2^32 and so on and therefore truncates all values to the range of -2147483647 and 2147483647.
Steps To Reproduceprintf("%d\n", __INT_MAX__) on 64-bit platforms.
TagsNo tags attached.
Attached Files
sprintf_64bit-V2.diff (1,957 bytes)   
Index: src/sprintf.c
===================================================================
--- src/sprintf.c	(Revision 2364)
+++ src/sprintf.c	(Arbeitskopie)
@@ -455,7 +455,7 @@
 
 /*-------------------------------------------------------------------------*/
 static void
-numadd (fmt_state_t *st, sprintf_buffer_t **buffer, int num)
+numadd (fmt_state_t *st, sprintf_buffer_t **buffer, p_int num)
 
 /* Add the <num>ber to the <buffer>.
  */
@@ -2243,7 +2243,7 @@
                   }
                   else
                   {
-                    char cheat[6];  /* Synthesized format for sprintf() */
+                    char cheat[8];  /* Synthesized format for sprintf() */
                     char temp[1024];
                       /* The buffer must be big enough to hold the biggest float
                        * in non-exponential representation. 1 KByte is hopefully
@@ -2305,12 +2305,22 @@
                          * strings can. So in that case we format with
                          * character 0x01 and convert to 0 afterwards.
                          */
-                        if (format_char == 'c' && carg->u.number == 0)
-                        {
+                        if (format_char == 'c') {
+                          if (carg->u.number == 0)
+                          {
                             carg->u.number = 1;
                             zeroCharHack = MY_TRUE;
+                          }
                         }
-
+                        else 
+                        {
+#if SIZEOF_LONG == SIZEOF_CHAR_P
+                          cheat[i++] = 'l';
+#elif defined(HAVE_LONG_LONG) && SIZEOF_LONG_LONG == SIZEOF_CHAR_P
+                          cheat[i++] = 'l';
+                          cheat[i++] = 'l';
+#endif
+                        }
                         cheat[i++] = format_char;
                         cheat[i] = '\0';
                         sprintf(temp, cheat, carg->u.number);
sprintf_64bit-V2.diff (1,957 bytes)   

Relationships

parent of 0000554 resolvedzesstra LDMud 3.3 port.h should use stdint.h, inttypes.h and stdbool.h if available 
related to 0000556 new LDMud 3.3 Change code which assumes that p_int and mp_int are always a long. 
child of 0000555 closed LDMud 3.5 Complete support for 64bit (LP64) architectures 

Activities

zesstra

2008-05-02 14:44

administrator   ~0000610

sprintf.c uses for T_FLOAT und T_NUMBER in most cases the system sprintf. Unfortunately, that needs a length modifier for long int ('l') and long long int ('ll'). I added some precompiler logic in string_print_formatted() around line 2314 to add 'l' or 'll' depending on the size of p_int (u.number in svalue).
That works, but has to 2 issues:
First it introduces stuff from port.h to sprintf.c, it would be nicer to keep that portability defines in port.h. Though I not sure, if it is practicable.
Second and more important, the format_char can be 'c' as well and '%lc' signifies a wide char wint_t. It would be needed to change that part of string_print_formatted() to use the 'l' and 'll' length mods only for integral conversions (d, i, o, u, x, or X). I still have to do that and it is sadly an additional runtime decision.

zesstra

2008-05-06 13:32

administrator   ~0000613

sprintf_64bit-V2.diff should fix the 2 obvious issues with sprintf() concerning 64-bit Ints and %d, %i, %x and %O.
1) change num_add(..., int num) to num_add(..., p_int num)
2) add 'l' or 'll' to %d, %i, %x (but not %c) before passing them on to the system lib.

There are still a lot of warnings "implicit conversion shortens 64-bit value into a 32-bit value" in sprintf.c left and some of them might well be a bug. But they have to wait for another patch.

zesstra

2008-09-06 16:19

administrator   ~0000762

Fixed in r2413.

Issue History

Date Modified Username Field Change
2008-01-27 16:04 zesstra New Issue
2008-04-01 08:20 zesstra File Added: sprintf_64bit.diff
2008-05-02 14:44 zesstra Note Added: 0000610
2008-05-02 15:38 zesstra File Added: sprintf_64bit-V2.diff
2008-05-06 13:32 zesstra Note Added: 0000613
2008-06-30 02:55 zesstra Status new => assigned
2008-06-30 02:55 zesstra Assigned To => zesstra
2008-07-08 15:25 zesstra File Deleted: sprintf_64bit.diff
2008-07-12 16:17 zesstra Relationship added parent of 0000554
2008-07-13 12:02 zesstra Relationship added child of 0000555
2008-07-14 14:17 zesstra Relationship added related to 0000556
2008-09-06 16:19 zesstra Status assigned => resolved
2008-09-06 16:19 zesstra Fixed in Version => 3.3.718
2008-09-06 16:19 zesstra Resolution open => fixed
2008-09-06 16:19 zesstra Note Added: 0000762
2010-11-16 09:42 zesstra Source_changeset_attached => ldmud.git master d15b8b12
2018-01-29 18:59 zesstra Source_changeset_attached => ldmud.git master d15b8b12
2018-01-29 21:57 zesstra Source_changeset_attached => ldmud.git master d15b8b12