View Issue Details

IDProjectCategoryView StatusLast Update
0000528LDMud 3.3Portabilitypublic2018-01-29 22: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.

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 16: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.

2008-05-02 17:38

 

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)   

zesstra

2008-05-06 15: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 18:19

administrator   ~0000762

Fixed in r2413.

Issue History

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