[PATCH] exec.c: print_section_info() format string fixes

[PATCH] exec.c: print_section_info() format string fixes

[Kevin Buettner]

On 64-bit targets, I found that the values printed by print_section_info()
were not being printed in their entirety.  I've just committed the patch
below to fix this problem.

As I look at it now, I see that the FIXME comment can go.  I'll zap it
in a moment.

	* exec.c (print_section_info): Select a format string to use with
	local_hex_string_custom() based upon the value of TARGET_ADDR_BIT.

Index: exec.c
===================================================================
RCS file: /cvs/src/src/gdb/exec.c,v
retrieving revision 1.23
diff -u -p -r1.23 exec.c
--- exec.c	12 Dec 2002 01:39:34 -0000	1.23
+++ exec.c	19 Dec 2002 18:44:40 -0000
@@ -545,6 +545,7 @@ void
 print_section_info (struct target_ops *t, bfd *abfd)
 {
   struct section_table *p;
+  char *fmt = TARGET_ADDR_BIT <= 32 ? "08l" : "016l";
 
   printf_filtered ("\t`%s', ", bfd_get_filename (abfd));
   wrap_here ("        ");
@@ -558,11 +559,11 @@ print_section_info (struct target_ops *t
   for (p = t->to_sections; p < t->to_sections_end; p++)
     {
       /* FIXME-32x64 need a print_address_numeric with field width */
-      printf_filtered ("\t%s", local_hex_string_custom ((unsigned long) p->addr, "08l"));
-      printf_filtered (" - %s", local_hex_string_custom ((unsigned long) p->endaddr, "08l"));
+      printf_filtered ("\t%s", local_hex_string_custom (p->addr, fmt));
+      printf_filtered (" - %s", local_hex_string_custom (p->endaddr, fmt));
       if (info_verbose)
 	printf_filtered (" @ %s",
-			 local_hex_string_custom ((unsigned long) p->the_bfd_section->filepos, "08l"));
+			 local_hex_string_custom (p->the_bfd_section->filepos, "08l"));
       printf_filtered (" is %s", bfd_section_name (p->bfd, p->the_bfd_section));
       if (p->bfd != abfd)
 	{

Re: [PATCH] exec.c: print_section_info() format string fixes

[Andrew Cagney]

> On 64-bit targets, I found that the values printed by print_section_info()
> were not being printed in their entirety.  I've just committed the patch
> below to fix this problem.
> 
> As I look at it now, I see that the FIXME comment can go.  I'll zap it
> in a moment.

I don't think the problem is fixed, just improved.  The 08l / 016l test 
assumes that the address is <= 64 bits.  Hence, I think some sort of 
FIXME should remain (look for other similar cases).  The bug database 
contains a suggestion that a new function - 
local_address_string_custom()(?) be added.


> 	* exec.c (print_section_info): Select a format string to use with
> 	local_hex_string_custom() based upon the value of TARGET_ADDR_BIT.
> 

>  	printf_filtered (" @ %s",
> -			 local_hex_string_custom ((unsigned long) p->the_bfd_section->filepos, "08l"));
> +			 local_hex_string_custom (p->the_bfd_section->filepos, "08l"));

BTW, Is the above correct?

Andrew

Re: [PATCH] exec.c: print_section_info() format string fixes

[Kevin Buettner]

On Dec 20,  6:51pm, Andrew Cagney wrote:

> > On 64-bit targets, I found that the values printed by print_section_info()
> > were not being printed in their entirety.  I've just committed the patch
> > below to fix this problem.
> > 
> > As I look at it now, I see that the FIXME comment can go.  I'll zap it
> > in a moment.
> 
> I don't think the problem is fixed, just improved.  The 08l / 016l test 
> assumes that the address is <= 64 bits.  Hence, I think some sort of 
> FIXME should remain (look for other similar cases).  The bug database 
> contains a suggestion that a new function - 
> local_address_string_custom()(?) be added.

Okay, I'll put some sort of FIXME back.

> > 	* exec.c (print_section_info): Select a format string to use with
> > 	local_hex_string_custom() based upon the value of TARGET_ADDR_BIT.
> > 
> 
> >  	printf_filtered (" @ %s",
> > -			 local_hex_string_custom ((unsigned long) p->the_bfd_section->filepos, "08l"));
> > +			 local_hex_string_custom (p->the_bfd_section->filepos, "08l"));
> 
> BTW, Is the above correct?

Sort of.

The cast is definitely not needed.

I had originally thought that "016l" should also be used for this, but
it makes the output a lot wider than I find desirable.  Ideally, I
guess we'd test to see how large the file is, and then pick a format
string based upon that.  I.e, if the file is bigger than 4GB, we'd use
"016l" for the file position.  But, since I've never seen an
executable or shared library bigger than 4GB, I'm not particularly
worried about it.

However, I'll add a FIXME before that line noting the potential
problem.

Re: [PATCH] exec.c: print_section_info() format string fixes

[Kevin Buettner]

On Dec 20,  6:51pm, Andrew Cagney wrote:

> I don't think the problem is fixed, just improved.  The 08l / 016l test 
> assumes that the address is <= 64 bits.  Hence, I think some sort of 
> FIXME should remain [...]

With regard to the FIXME comments, I've just committed the following:

	* exec.c (print_section_info): Add FIXME comments regarding format
	string choices.

Index: exec.c
===================================================================
RCS file: /cvs/src/src/gdb/exec.c,v
retrieving revision 1.25
diff -u -p -r1.25 exec.c
--- exec.c	19 Dec 2002 19:02:57 -0000	1.25
+++ exec.c	23 Dec 2002 23:34:24 -0000
@@ -545,6 +545,7 @@ void
 print_section_info (struct target_ops *t, bfd *abfd)
 {
   struct section_table *p;
+  /* FIXME: "016l" is not wide enough when TARGET_ADDR_BIT > 64.  */
   char *fmt = TARGET_ADDR_BIT <= 32 ? "08l" : "016l";
 
   printf_filtered ("\t`%s', ", bfd_get_filename (abfd));
@@ -560,6 +561,12 @@ print_section_info (struct target_ops *t
     {
       printf_filtered ("\t%s", local_hex_string_custom (p->addr, fmt));
       printf_filtered (" - %s", local_hex_string_custom (p->endaddr, fmt));
+
+      /* FIXME: A format of "08l" is not wide enough for file offsets
+	 larger than 4GB.  OTOH, making it "016l" isn't desirable either
+	 since most output will then be much wider than necessary.  It
+	 may make sense to test the size of the file and choose the
+	 format string accordingly.  */
       if (info_verbose)
 	printf_filtered (" @ %s",
 			 local_hex_string_custom (p->the_bfd_section->filepos, "08l"));

Re: [PATCH] exec.c: print_section_info() format string fixes

[Andrew Cagney]

> On Dec 20,  6:51pm, Andrew Cagney wrote:
> 
> 
>> I don't think the problem is fixed, just improved.  The 08l / 016l test 
>> assumes that the address is <= 64 bits.  Hence, I think some sort of 
>> FIXME should remain [...]
> 
> 
> With regard to the FIXME comments, I've just committed the following:
> 
> 	* exec.c (print_section_info): Add FIXME comments regarding format
> 	string choices.


thanks!

Andrew


> Index: exec.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/exec.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 exec.c
> --- exec.c	19 Dec 2002 19:02:57 -0000	1.25
> +++ exec.c	23 Dec 2002 23:34:24 -0000
> @@ -545,6 +545,7 @@ void
>  print_section_info (struct target_ops *t, bfd *abfd)
>  {
>    struct section_table *p;
> +  /* FIXME: "016l" is not wide enough when TARGET_ADDR_BIT > 64.  */
>    char *fmt = TARGET_ADDR_BIT <= 32 ? "08l" : "016l";
>  
>    printf_filtered ("\t`%s', ", bfd_get_filename (abfd));
> @@ -560,6 +561,12 @@ print_section_info (struct target_ops *t
>      {
>        printf_filtered ("\t%s", local_hex_string_custom (p->addr, fmt));
>        printf_filtered (" - %s", local_hex_string_custom (p->endaddr, fmt));
> +
> +      /* FIXME: A format of "08l" is not wide enough for file offsets
> +	 larger than 4GB.  OTOH, making it "016l" isn't desirable either
> +	 since most output will then be much wider than necessary.  It
> +	 may make sense to test the size of the file and choose the
> +	 format string accordingly.  */
>        if (info_verbose)
>  	printf_filtered (" @ %s",
>  			 local_hex_string_custom (p->the_bfd_section->filepos, "08l"));
> 
>