diff mbox

constify to_attach

Message ID 1400696455-29563-1-git-send-email-tromey@redhat.com
State Superseded
Headers

Commit Message

Tom Tromey May 21, 2014, 6:20 p.m. UTC 
  This constifies the "args" argument to the target_ops to_attach
method.

I updated all instances of the method.  I could not compile all of
them but I hand-inspected them.  In all cases either the argument is
ignored, or it is passed to parse_pid_to_attach.  (linux-nat does some
extra stuff, but that one I built...)

If you want to try it on your host of choice, please do so.

The code in parse_pid_to_attach seems a little bogus to me.  If there
is a platform with a broken strtoul, we have better methods for fixing
the issue now.  However, I left the code as is since it is clearly ok
to do so.

Built and regtested on x86-64 Fedora 20.

2014-05-21  Tom Tromey  <tromey@redhat.com>

	* windows-nat.c (windows_attach): Make "args" const.
	* nto-procfs.c (procfs_attach): Make "args" const.
	* inf-ttrace.c (inf_ttrace_attach): Make "args" const.
	* go32-nat.c (go32_attach): Make "args" const.
	* gnu-nat.c (gnu_attach): Make "args" const.
	* darwin-nat.c (darwin_attach): Make "args" const.
	* inf-ptrace.c (inf_ptrace_attach): Make "args" const.
	* linux-nat.c (linux_nat_attach): Make "args" const.
	* remote.c (extended_remote_attach_1, extended_remote_attach):
	Make "args" const.
	* target.h (struct target_ops) <to_attach>: Make "args" const.
	(find_default_attach): Likewise.
	* utils.c (parse_pid_to_attach): Make "args" const.
	* utils.h (parse_pid_to_attach): Update.
---
 gdb/ChangeLog     | 17 +++++++++++++++++
 gdb/darwin-nat.c  |  2 +-
 gdb/gnu-nat.c     |  2 +-
 gdb/go32-nat.c    |  2 +-
 gdb/inf-ptrace.c  |  2 +-
 gdb/inf-ttrace.c  |  2 +-
 gdb/linux-nat.c   |  2 +-
 gdb/nto-procfs.c  |  2 +-
 gdb/remote.c      |  5 +++--
 gdb/target.h      |  4 ++--
 gdb/utils.c       |  4 ++--
 gdb/utils.h       |  2 +-
 gdb/windows-nat.c |  2 +-
 13 files changed, 33 insertions(+), 15 deletions(-)

Comments

Pedro Alves May 21, 2014, 7:45 p.m. UTC | #1 
On 05/21/2014 07:20 PM, Tom Tromey wrote:

> The code in parse_pid_to_attach seems a little bogus to me.  If there
> is a platform with a broken strtoul, we have better methods for fixing
> the issue now.  However, I left the code as is since it is clearly ok
> to do so.

Yeah.  We should probably use get_number or some such that accepts
convenience vars even.

> +parse_pid_to_attach (const char *args)
>  {
>    unsigned long pid;
>    char *dummy;
> @@ -3257,7 +3257,7 @@ parse_pid_to_attach (char *args)
>    if (!args)
>      error_no_arg (_("process-id to attach"));
>  
> -  dummy = args;
> +  dummy = (char *) args;
>    pid = strtoul (args, &dummy, 0);
>    /* Some targets don't set errno on errors, grrr!  */
>    if ((pid == 0 && dummy == args) || dummy != &args[strlen (args)])

errno would be necessary to catch overflow, but not to check whether
the number was syntactically correct.  strtoul always sets *endptr to
point to the address of the first invalid character (and never to NULL).

So you could just remove the 'dummy' assignment.

But I'll understand if you want to keep it.

The patch looks fine to me.
Tom Tromey June 4, 2014, 5:09 p.m. UTC | #2 
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> -  dummy = args;
>> +  dummy = (char *) args;
>> pid = strtoul (args, &dummy, 0);
>> /* Some targets don't set errno on errors, grrr!  */
>> if ((pid == 0 && dummy == args) || dummy != &args[strlen (args)])

Pedro> errno would be necessary to catch overflow, but not to check whether
Pedro> the number was syntactically correct.  strtoul always sets *endptr to
Pedro> point to the address of the first invalid character (and never to NULL).

Pedro> So you could just remove the 'dummy' assignment.

Pedro> But I'll understand if you want to keep it.

I agree that the assignment is not necessary.

However, I left it since presumably it is based on some ancient, broken
strtoul where it was actually needed, and I didn't want to get into this
aspect of the code.

Pedro> The patch looks fine to me.

I'm pushing it now.

Tom
diff mbox

Patch

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index f3d510d..51087ea 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1673,7 +1673,7 @@  darwin_setup_fake_stop_event (struct inferior *inf)
 /* Attach to process PID, then initialize for debugging it
    and wait for the trace-trap that results from attaching.  */
 static void
-darwin_attach (struct target_ops *ops, char *args, int from_tty)
+darwin_attach (struct target_ops *ops, const char *args, int from_tty)
 {
   pid_t pid;
   pid_t pid2;
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 3317215..5177ac0 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2166,7 +2166,7 @@  gnu_create_inferior (struct target_ops *ops,
 /* Attach to process PID, then initialize for debugging it
    and wait for the trace-trap that results from attaching.  */
 static void
-gnu_attach (struct target_ops *ops, char *args, int from_tty)
+gnu_attach (struct target_ops *ops, const char *args, int from_tty)
 {
   int pid;
   char *exec_file;
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index 77843ea..c7a5e5a 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -339,7 +339,7 @@  static struct {
 };
 
 static void
-go32_attach (struct target_ops *ops, char *args, int from_tty)
+go32_attach (struct target_ops *ops, const char *args, int from_tty)
 {
   error (_("\
 You cannot attach to a running program on this platform.\n\
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index cc4921b..4c1a18c 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -183,7 +183,7 @@  inf_ptrace_mourn_inferior (struct target_ops *ops)
    be chatty about it.  */
 
 static void
-inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty)
+inf_ptrace_attach (struct target_ops *ops, const char *args, int from_tty)
 {
   char *exec_file;
   pid_t pid;
diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
index 96105dc..406ec26 100644
--- a/gdb/inf-ttrace.c
+++ b/gdb/inf-ttrace.c
@@ -748,7 +748,7 @@  inf_ttrace_create_threads_after_attach (int pid)
 }
 
 static void
-inf_ttrace_attach (struct target_ops *ops, char *args, int from_tty)
+inf_ttrace_attach (struct target_ops *ops, const char *args, int from_tty)
 {
   char *exec_file;
   pid_t pid;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index f60d54c..ff4dff6 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1300,7 +1300,7 @@  linux_nat_create_inferior (struct target_ops *ops,
 }
 
 static void
-linux_nat_attach (struct target_ops *ops, char *args, int from_tty)
+linux_nat_attach (struct target_ops *ops, const char *args, int from_tty)
 {
   struct lwp_info *lp;
   int status;
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 8a241a8..0d0197d 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -602,7 +602,7 @@  procfs_files_info (struct target_ops *ignore)
 
 /* Attach to process PID, then initialize for debugging it.  */
 static void
-procfs_attach (struct target_ops *ops, char *args, int from_tty)
+procfs_attach (struct target_ops *ops, const char *args, int from_tty)
 {
   char *exec_file;
   int pid;
diff --git a/gdb/remote.c b/gdb/remote.c
index 964bd41..394b58e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4364,7 +4364,8 @@  remote_disconnect (struct target_ops *target, char *args, int from_tty)
    be chatty about it.  */
 
 static void
-extended_remote_attach_1 (struct target_ops *target, char *args, int from_tty)
+extended_remote_attach_1 (struct target_ops *target, const char *args,
+			  int from_tty)
 {
   struct remote_state *rs = get_remote_state ();
   int pid;
@@ -4477,7 +4478,7 @@  extended_remote_attach_1 (struct target_ops *target, char *args, int from_tty)
 }
 
 static void
-extended_remote_attach (struct target_ops *ops, char *args, int from_tty)
+extended_remote_attach (struct target_ops *ops, const char *args, int from_tty)
 {
   extended_remote_attach_1 (ops, args, from_tty);
 }
diff --git a/gdb/target.h b/gdb/target.h
index 9371529..e841ffb 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -409,7 +409,7 @@  struct target_ops
        for normal operations, and should be ready to deliver the
        status of the process immediately (without waiting) to an
        upcoming target_wait call.  */
-    void (*to_attach) (struct target_ops *ops, char *, int);
+    void (*to_attach) (struct target_ops *ops, const char *, int);
     void (*to_post_attach) (struct target_ops *, int)
       TARGET_DEFAULT_IGNORE ();
     void (*to_detach) (struct target_ops *ops, const char *, int)
@@ -2140,7 +2140,7 @@  extern void noprocess (void) ATTRIBUTE_NORETURN;
 
 extern void target_require_runnable (void);
 
-extern void find_default_attach (struct target_ops *, char *, int);
+extern void find_default_attach (struct target_ops *, const char *, int);
 
 extern void find_default_create_inferior (struct target_ops *,
 					  char *, char *, char **, int);
diff --git a/gdb/utils.c b/gdb/utils.c
index a8a7cb3..7506c37 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3249,7 +3249,7 @@  gdb_bfd_errmsg (bfd_error_type error_tag, char **matching)
 /* Return ARGS parsed as a valid pid, or throw an error.  */
 
 int
-parse_pid_to_attach (char *args)
+parse_pid_to_attach (const char *args)
 {
   unsigned long pid;
   char *dummy;
@@ -3257,7 +3257,7 @@  parse_pid_to_attach (char *args)
   if (!args)
     error_no_arg (_("process-id to attach"));
 
-  dummy = args;
+  dummy = (char *) args;
   pid = strtoul (args, &dummy, 0);
   /* Some targets don't set errno on errors, grrr!  */
   if ((pid == 0 && dummy == args) || dummy != &args[strlen (args)])
diff --git a/gdb/utils.h b/gdb/utils.h
index 33371ac..0eba8ae 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -63,7 +63,7 @@  struct timeval get_prompt_for_continue_wait_time (void);
 
 /* Parsing utilites.  */
 
-extern int parse_pid_to_attach (char *args);
+extern int parse_pid_to_attach (const char *args);
 
 extern int parse_escape (struct gdbarch *, const char **);
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index fe43c24..a2a95e5 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1833,7 +1833,7 @@  out:
 
 /* Attach to process PID, then initialize for debugging it.  */
 static void
-windows_attach (struct target_ops *ops, char *args, int from_tty)
+windows_attach (struct target_ops *ops, const char *args, int from_tty)
 {
   BOOL ok;
   DWORD pid;