Re: [PATCH 2/2] Fix ZERO_FILL flag to mkstring()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




----- Original Message -----
> Dne stÅeda 02 Ãnor 2011 11:43:18 Petr Tesarik napsal(a):
> > The ZERO_FILL flag should in fact be honoured during justification,
> > not for the formatting. This patch makes it possible to use
> > the ZERO_FILL flag for any type, not just LONG_DEC.
> >
> > Signed-off-by: Petr Tesarik <ptesarik@xxxxxxx>
> 
> Argh, scrath this. I only re-compiled tools.c to see if it works, but there
> are more callers of shift_string_right().

Any function that is exported in defs.h must remain there -- with the
same prototype -- because they could be used by a pre-existing extension
module.
 
> Fixed version attached.

Quickly looking at the patch I didn't quite understand how/why this
part could be removed:

@@ -1562,12 +1573,6 @@ mkstring(char *s, int size, ulong flags,
 	case LONG_HEX:
 		sprintf(s, "%lx", (ulong)opt);
 		break;
-	case (LONG_HEX|ZERO_FILL):
-		if (VADDR_PRLEN == 8)
-			sprintf(s, "%08lx", (ulong)opt);
-		else if (VADDR_PRLEN == 16)
-			sprintf(s, "%016lx", (ulong)opt);
-		break;
 	case INT_DEC:
 		sprintf(s, "%u", (uint)((ulong)opt));
 		break;

so I applied this patch to test.c to check out an example of
LONG_HEX|ZERO_FILL:

--- test.c.orig 2011-02-02 10:30:31.000000000 -0500
+++ test.c      2011-02-02 10:27:11.000000000 -0500
@@ -42,6 +42,15 @@
                 ;
                 optind++;
         }
+
+{
+       char buf[BUFSIZE];
+       ulong inode;
+       inode = 1;
+
+       fprintf(fp, "%s\n", mkstring(buf, VADDR_PRLEN,
+               CENTER|LONG_HEX|ZERO_FILL, MKSTR(inode)));
+}
 }

It should simply zero-fill the long hexadecimal value,
and should show this on a 64-bit machine:

  crash> test
  0000000000000001
  crash>

But with your patch applied, it fails like this:

  crash> test
  0000000100000000
  crash>

Again, I get really nervous when you start fixing things
without there being a bug there to begin with...

Dave




--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility



[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux