Re: [PATCH]: grub: Fix handling of long printf arguments on 64-bit.

[phcoder <phcoder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3898 bytes --]

I already understood what you meant in first mail. Sorry for not paying 
attention to this detail. Here is my proposition. IT decreases the size 
from 31224 to 31068 bytes. I tested it with following input
   grub_printf ("Hello World: %d %ld %lld %x %lx %llx %u %lu %llu\n",
                0xffffffff, 0xffffffff, 0xffffffffffffffffLL,
                0xffffffff, 0xffffffff, 0xffffffffffffffffLL,
                0xffffffff, 0xffffffff, 0xffffffffffffffffLL);
   grub_printf ("Hello World: %d %ld %lld %x %lx %llx %u %lu %llu\n",
                0x0fffffff, 0x0fffffff, 0x0fffffffffffffffLL,
                0x0fffffff, 0x0fffffff, 0x0fffffffffffffffLL,
                0x0fffffff, 0x0fffffff, 0x0fffffffffffffffLL);
Output was:
Hello World: -1 -1 -1 ffffffff ffffffff ffffffffffffffff 4294967295 
4294967295 18446744073709551615
Hello World: 268435455 268435455 1152921504606846975 fffffff fffffff 
fffffffffffffff 268435455 268435455 1152921504606846975

Pavel Roskin wrote:
> On Sun, 2009-04-12 at 17:33 -0400, Pavel Roskin wrote:
> 
>> If the type is going to be converted, we need to treat signed and
>> unsigned values separately.
> 
> This is compile tested only, to show what I mean.  The size of
> kernel.img is reduced from 31220 to 31124.  The code looks long, but I'm
> sure an optimizing compiler can simplify it well.  Variable names are
> changed for readability.
> 
> Index: kern/misc.c
> ===================================================================
> --- kern/misc.c	(revision 2087)
> +++ kern/misc.c	(working copy)
> @@ -565,56 +565,6 @@
>      }
>  }
>  
> -static char *
> -grub_itoa (char *str, int c, unsigned n)
> -{
> -  unsigned base = (c == 'x') ? 16 : 10;
> -  char *p;
> -  
> -  if ((int) n < 0 && c == 'd')
> -    {
> -      n = (unsigned) (-((int) n));
> -      *str++ = '-';
> -    }
> -
> -  p = str;
> -  do
> -    {
> -      unsigned d = n % base;
> -      *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
> -    }
> -  while (n /= base);
> -  *p = 0;
> -
> -  grub_reverse (str);
> -  return p;
> -}
> -
> -static char *
> -grub_ltoa (char *str, int c, unsigned long n)
> -{
> -  unsigned long base = (c == 'x') ? 16 : 10;
> -  char *p;
> -
> -  if ((long) n < 0 && c == 'd')
> -    {
> -      n = (unsigned long) (-((long) n));
> -      *str++ = '-';
> -    }
> -
> -  p = str;
> -  do
> -    {
> -      unsigned long d = n % base;
> -      *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
> -    }
> -  while (n /= base);
> -  *p = 0;
> -
> -  grub_reverse (str);
> -  return p;
> -}
> -
>  /* Divide N by D, return the quotient, and store the remainder in *R.  */
>  grub_uint64_t
>  grub_divmod64 (grub_uint64_t n, grub_uint32_t d, grub_uint32_t *r)
> @@ -807,23 +757,36 @@
>  	      /* fall through */
>  	    case 'x':
>  	    case 'u':
> +	      if (longlongfmt)
> +		{
> +		  unsigned long long ull = va_arg (args, unsigned long long);
> +		  grub_lltoa (tmp, c, ull);
> +		}
> +	      else if (longfmt)
> +		{
> +		  unsigned long ul = va_arg (args, unsigned long);
> +		  grub_lltoa (tmp, c, ul);
> +		}
> +	      else
> +		{
> +		  unsigned int ui = va_arg (args, unsigned int);
> +		  grub_lltoa (tmp, c, ui);
> +		}
>  	    case 'd':
>  	      if (longlongfmt)
>  		{
> -		  long long ll;
> -
> -		  ll = va_arg (args, long long);
> -		  grub_lltoa (tmp, c, ll);
> +		  long long sll = va_arg (args, long long);
> +		  grub_lltoa (tmp, c, sll);
>  		}
>  	      else if (longfmt)
>  		{
> -		  long l = va_arg (args, long);
> -		  grub_ltoa (tmp, c, l);
> +		  long sl = va_arg (args, long);
> +		  grub_lltoa (tmp, c, sl);
>  		}
>  	      else
>  		{
> -		  n = va_arg (args, int);
> -		  grub_itoa (tmp, c, n);
> +		  int si = va_arg (args, int);
> +		  grub_lltoa (tmp, c, si);
>  		}
>  	      if (! rightfill && grub_strlen (tmp) < format1)
>  		write_fill (zerofill, format1 - grub_strlen (tmp));
> 
> 


-- 

Regards
Vladimir 'phcoder' Serbinenko

[-- Attachment #2: lltoa.diff --]
[-- Type: text/x-diff, Size: 2907 bytes --]

diff --git a/ChangeLog b/ChangeLog
index 237c6f4..d701519 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2009-04-13  Vladimir Serbinenko	 <phcoder@gmail.com>
+
+	use grub_lltoa instead of grub_itoa and grub_ltoa for all purposes
+	
+	* kern/misc.c (grub_itoa): Removed function
+	(grub_ltoa): likewise
+	(grub_vsprintf): use grub_lltoa
+
 2009-04-12  David S. Miller  <davem@davemloft.net>
 
 	* kern/misc.c (grub_ltoa): Fix cast when handling negative
diff --git a/kern/misc.c b/kern/misc.c
index 5a12535..4c71ca8 100644
--- a/kern/misc.c
+++ b/kern/misc.c
@@ -565,56 +565,6 @@ grub_reverse (char *str)
     }
 }
 
-static char *
-grub_itoa (char *str, int c, unsigned n)
-{
-  unsigned base = (c == 'x') ? 16 : 10;
-  char *p;
-  
-  if ((int) n < 0 && c == 'd')
-    {
-      n = (unsigned) (-((int) n));
-      *str++ = '-';
-    }
-
-  p = str;
-  do
-    {
-      unsigned d = n % base;
-      *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
-    }
-  while (n /= base);
-  *p = 0;
-
-  grub_reverse (str);
-  return p;
-}
-
-static char *
-grub_ltoa (char *str, int c, unsigned long n)
-{
-  unsigned long base = (c == 'x') ? 16 : 10;
-  char *p;
-
-  if ((long) n < 0 && c == 'd')
-    {
-      n = (unsigned long) (-((long) n));
-      *str++ = '-';
-    }
-
-  p = str;
-  do
-    {
-      unsigned long d = n % base;
-      *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
-    }
-  while (n /= base);
-  *p = 0;
-
-  grub_reverse (str);
-  return p;
-}
-
 /* Divide N by D, return the quotient, and store the remainder in *R.  */
 grub_uint64_t
 grub_divmod64 (grub_uint64_t n, grub_uint32_t d, grub_uint32_t *r)
@@ -746,6 +696,7 @@ grub_vsprintf (char *str, const char *fmt, va_list args)
 	  int n;
 	  int longfmt = 0;
 	  int longlongfmt = 0;
+	  int unsig = 0;
 
 	  if (*fmt && *fmt =='-')
 	    {
@@ -804,9 +755,11 @@ grub_vsprintf (char *str, const char *fmt, va_list args)
 	      write_str ("0x");
 	      c = 'x';
 	      longlongfmt |= (sizeof (void *) == sizeof (long long));
-	      /* fall through */
+	      /* Fall through. */
 	    case 'x':
 	    case 'u':
+	      unsig = 1;
+	      /* Fall through. */
 	    case 'd':
 	      if (longlongfmt)
 		{
@@ -815,15 +768,25 @@ grub_vsprintf (char *str, const char *fmt, va_list args)
 		  ll = va_arg (args, long long);
 		  grub_lltoa (tmp, c, ll);
 		}
+	      else if (longfmt && unsig)
+		{
+		  unsigned long l = va_arg (args, unsigned long);
+		  grub_lltoa (tmp, c, l);
+		}
 	      else if (longfmt)
 		{
 		  long l = va_arg (args, long);
-		  grub_ltoa (tmp, c, l);
+		  grub_lltoa (tmp, c, l);
+		}
+	      else if (unsig)
+		{
+		  unsigned u = va_arg (args, unsigned);
+		  grub_lltoa (tmp, c, u);
 		}
 	      else
 		{
 		  n = va_arg (args, int);
-		  grub_itoa (tmp, c, n);
+		  grub_lltoa (tmp, c, n);
 		}
 	      if (! rightfill && grub_strlen (tmp) < format1)
 		write_fill (zerofill, format1 - grub_strlen (tmp));