Re: [patch 1/2+7.6] /proc/PID/smaps: filename fix

Re: [patch 1/2+7.6] /proc/PID/smaps: filename fix

[Andreas Schwab]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> currently GDB complains:
> 	warning: Error parsing {s,}maps file '[heap]'
> 	warning: Error parsing {s,}maps file '/usr/lib64/libc-2.17.so'
>
> which is sure incorrect, it should have been:
> 	warning: Error parsing {s,}maps file '/proc/30100/smaps'
> 	warning: Error parsing {s,}maps file '/proc/30100/smaps'

Which mean you'll no longer know what's wrong with it.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[patch 1/2+7.6] /proc/PID/smaps: filename fix

[Jan Kratochvil]

Hi,

currently GDB complains:
	warning: Error parsing {s,}maps file '[heap]'
	warning: Error parsing {s,}maps file '/usr/lib64/libc-2.17.so'

which is sure incorrect, it should have been:
	warning: Error parsing {s,}maps file '/proc/30100/smaps'
	warning: Error parsing {s,}maps file '/proc/30100/smaps'

(That the error happens at all is off-topic for this mail.)

While it is 'obvious' GDB CVS inaccessible to me now so neither regression
tested nor checked in yet.


Thanks,
Jan


gdb/
2013-03-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix variable name shadowing.
	* linux-tdep.c (linux_find_memory_regions_full): Rename outer variable
	filename to mapsfilename and update its uses.

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index a132fc6..cc63e9b 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -675,22 +675,22 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 				linux_find_memory_region_ftype *func,
 				void *obfd)
 {
-  char filename[100];
+  char mapsfilename[100];
   gdb_byte *data;
 
   /* We need to know the real target PID to access /proc.  */
   if (current_inferior ()->fake_pid_p)
     return 1;
 
-  xsnprintf (filename, sizeof filename,
+  xsnprintf (mapsfilename, sizeof mapsfilename,
 	     "/proc/%d/smaps", current_inferior ()->pid);
-  data = target_fileio_read_stralloc (filename);
+  data = target_fileio_read_stralloc (mapsfilename);
   if (data == NULL)
     {
       /* Older Linux kernels did not support /proc/PID/smaps.  */
-      xsnprintf (filename, sizeof filename,
+      xsnprintf (mapsfilename, sizeof mapsfilename,
 		 "/proc/%d/maps", current_inferior ()->pid);
-      data = target_fileio_read_stralloc (filename);
+      data = target_fileio_read_stralloc (mapsfilename);
     }
   if (data)
     {
@@ -724,7 +724,7 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 
 	      if (sscanf (line, "%64s%lu kB\n", keyword, &number) != 2)
 		{
-		  warning (_("Error parsing {s,}maps file '%s'"), filename);
+		  warning (_("Error parsing {s,}maps file '%s'"), mapsfilename);
 		  break;
 		}
 	      if (strcmp (keyword, "Anonymous:") == 0)

Re: [patch 1/2+7.6] /proc/PID/smaps: filename fix

[Pedro Alves]

On 03/29/2013 08:27 PM, Jan Kratochvil wrote:
> On Fri, 29 Mar 2013 21:24:38 +0100, Andreas Schwab wrote:
>> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>>
>>> currently GDB complains:
>>> 	warning: Error parsing {s,}maps file '[heap]'
>>> 	warning: Error parsing {s,}maps file '/usr/lib64/libc-2.17.so'
>>>
>>> which is sure incorrect, it should have been:
>>> 	warning: Error parsing {s,}maps file '/proc/30100/smaps'
>>> 	warning: Error parsing {s,}maps file '/proc/30100/smaps'
>>
>> Which mean you'll no longer know what's wrong with it.

The current warning doesn't actually show what failed to parse.
We'd need to print the line for that.

> 
> I believe the second output was originally intended, this is what this patch
> tries to fix.

I agree.

> 
> It may not be perfect but it only tries to suggest what is that "{s,}maps
> file".  I did not write this code (although reviewed it IIRC).


-- 
Pedro Alves

[commit+7.6] [patch 1/2+7.6] /proc/PID/smaps: filename fix

[Jan Kratochvil]

On Fri, 05 Apr 2013 20:30:39 +0200, Pedro Alves wrote:
> The current warning doesn't actually show what failed to parse.
> We'd need to print the line for that.

I will post a new patch for it.


> > I believe the second output was originally intended, this is what this patch
> > tries to fix.
> 
> I agree.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-04/msg00052.html
and for 7.6:
	http://sourceware.org/ml/gdb-cvs/2013-04/msg00053.html


Thanks,
Jan