Re: [RFA] linux-nat.c minor cleanup

[Doug Evans <dje@google.com>]

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

On Mon, Jan 5, 2009 at 3:04 PM, Doug Evans <dje@google.com> wrote:
> Ping.
>
> On Thu, Dec 18, 2008 at 9:34 PM, Doug Evans <dje@google.com> wrote:
>> Hi.
>>
>> linux-nat.c:linux_nat_info_proc_cmd uses a long long to record a pid.
>> There's not much point in that, so this patch changes it to a long.
>> [either that or it shouldn't use strtoul to parse it :-)]
>>
>> The handling of info proc cmd,cwd,exe is odd too.
>> They're always printed and yet argument parsing looks for them anyway,
>> and ignores invalid commands instead of flagging an error.
>> This patch adds a blurb about printing cmd,cwd,exe in the help text.
>> The manual already touches on this, though it doesn't precisely
>> say that cmd,cwd,exe are always printed.
>> I wouldn't mind also submitting a patch to flag unknown commands
>> as errors, but I'm not sure what's intended as far as the parsing
>> of cmd,cwd,exe.

Going over the file I found a case I missed.
atoi has undefined behaviour for invalid input so I stuck with strtoul
in linux_nat_info_proc_cmd.  Granted, it is awkward that some fns use
long and some fns use int, but ptid_get_pid returns an int and strotul
returns a long.  strtopid anyone?

[I realize this is just minor cleanup, but for completeness' sake,
there are several uses of pid_t in linux-nat.c - maybe pid_t should be
used everywhere instead of int - if one wants to be pedantic.  But
then one is left with either assuming it's an int or adding int casts
to sprintf arguments (which the code already does).  I don't have a
preference, I'll make the patch look like whatever y'all want.  It's
just annoying to see long long used here.]

This version removes the change to the doc string for "info proc".  I
could still use some input on what preferred: remove the argument
processing to watch for cmd, cwd, exe?

2008-12-18  Doug Evans  <dje@google.com>

        * linux-nat.c (linux_nat_find_memory_regions): Result of PIDGET is an
        int, not a long long.
        (linux_nat_info_proc_cmd): Store pid in long instead of long long.

[-- Attachment #2: gdb-090105-linux-nat-1.patch.txt --]
[-- Type: text/plain, Size: 3865 bytes --]

2008-12-18  Doug Evans  <dje@google.com>

	* linux-nat.c (linux_nat_find_memory_regions): Result of PIDGET is an
	int, not a long long.
	(linux_nat_info_proc_cmd): Store pid in long instead of long long.

Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.115
diff -u -p -u -p -r1.115 linux-nat.c
--- linux-nat.c	3 Jan 2009 05:57:52 -0000	1.115
+++ linux-nat.c	5 Jan 2009 23:41:22 -0000
@@ -3346,7 +3346,7 @@ linux_nat_find_memory_regions (int (*fun
 					    unsigned long,
 					    int, int, int, void *), void *obfd)
 {
-  long long pid = PIDGET (inferior_ptid);
+  int pid = PIDGET (inferior_ptid);
   char mapsfilename[MAXPATHLEN];
   FILE *mapsfile;
   long long addr, endaddr, size, offset, inode;
@@ -3356,7 +3356,7 @@ linux_nat_find_memory_regions (int (*fun
   struct cleanup *cleanup;
 
   /* Compose the filename for the /proc memory map, and open it.  */
-  sprintf (mapsfilename, "/proc/%lld/maps", pid);
+  sprintf (mapsfilename, "/proc/%d/maps", pid);
   if ((mapsfile = fopen (mapsfilename, "r")) == NULL)
     error (_("Could not open %s."), mapsfilename);
   cleanup = make_cleanup_fclose (mapsfile);
@@ -3609,7 +3609,9 @@ linux_nat_make_corefile_notes (bfd *obfd
 static void
 linux_nat_info_proc_cmd (char *args, int from_tty)
 {
-  long long pid = PIDGET (inferior_ptid);
+  /* A long is used for pid instead of an int to avoid a loss of precision
+     compiler warning from the output of strtoul.  */
+  long pid = PIDGET (inferior_ptid);
   FILE *procfile;
   char **argv = NULL;
   char buffer[MAXPATHLEN];
@@ -3673,14 +3675,14 @@ linux_nat_info_proc_cmd (char *args, int
   if (pid == 0)
     error (_("No current process: you must name one."));
 
-  sprintf (fname1, "/proc/%lld", pid);
+  sprintf (fname1, "/proc/%ld", pid);
   if (stat (fname1, &dummy) != 0)
     error (_("No /proc directory: '%s'"), fname1);
 
-  printf_filtered (_("process %lld\n"), pid);
+  printf_filtered (_("process %ld\n"), pid);
   if (cmdline_f || all)
     {
-      sprintf (fname1, "/proc/%lld/cmdline", pid);
+      sprintf (fname1, "/proc/%ld/cmdline", pid);
       if ((procfile = fopen (fname1, "r")) != NULL)
 	{
 	  struct cleanup *cleanup = make_cleanup_fclose (procfile);
@@ -3693,7 +3695,7 @@ linux_nat_info_proc_cmd (char *args, int
     }
   if (cwd_f || all)
     {
-      sprintf (fname1, "/proc/%lld/cwd", pid);
+      sprintf (fname1, "/proc/%ld/cwd", pid);
       memset (fname2, 0, sizeof (fname2));
       if (readlink (fname1, fname2, sizeof (fname2)) > 0)
 	printf_filtered ("cwd = '%s'\n", fname2);
@@ -3702,7 +3704,7 @@ linux_nat_info_proc_cmd (char *args, int
     }
   if (exe_f || all)
     {
-      sprintf (fname1, "/proc/%lld/exe", pid);
+      sprintf (fname1, "/proc/%ld/exe", pid);
       memset (fname2, 0, sizeof (fname2));
       if (readlink (fname1, fname2, sizeof (fname2)) > 0)
 	printf_filtered ("exe = '%s'\n", fname2);
@@ -3711,7 +3713,7 @@ linux_nat_info_proc_cmd (char *args, int
     }
   if (mappings_f || all)
     {
-      sprintf (fname1, "/proc/%lld/maps", pid);
+      sprintf (fname1, "/proc/%ld/maps", pid);
       if ((procfile = fopen (fname1, "r")) != NULL)
 	{
 	  long long addr, endaddr, size, offset, inode;
@@ -3773,7 +3775,7 @@ linux_nat_info_proc_cmd (char *args, int
     }
   if (status_f || all)
     {
-      sprintf (fname1, "/proc/%lld/status", pid);
+      sprintf (fname1, "/proc/%ld/status", pid);
       if ((procfile = fopen (fname1, "r")) != NULL)
 	{
 	  struct cleanup *cleanup = make_cleanup_fclose (procfile);
@@ -3786,7 +3788,7 @@ linux_nat_info_proc_cmd (char *args, int
     }
   if (stat_f || all)
     {
-      sprintf (fname1, "/proc/%lld/stat", pid);
+      sprintf (fname1, "/proc/%ld/stat", pid);
       if ((procfile = fopen (fname1, "r")) != NULL)
 	{
 	  int itmp;