diff mbox

constify to_attach

Message ID 87mwebhsc2.fsf@fleche.redhat.com
State Committed
Headers

Commit Message

Tom Tromey May 21, 2014, 6:58 p.m. UTC 
  >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> This constifies the "args" argument to the target_ops to_attach
Tom> method.

Tom> I updated all instances of the method.

.. except procfs.c, which I somehow missed.  Here's the updated patch.

I also wanted to note that I checked all the functions for forward
declarations as well.

Tom

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

	* procfs.c (procfs_attach): Make "args" const.
	* 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.

commit 2f15772fddec94e04478a52d63e333e53e4d4bf5
Author: Tom Tromey <tromey@redhat.com>
Date:   Mon Apr 15 09:40:57 2013 -0600

    constify to_attach
    
    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>
    
    	* procfs.c (procfs_attach): Make "args" const.
    	* 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.

Comments

Joel Brobecker May 21, 2014, 9:22 p.m. UTC | #1 
> I also wanted to note that I checked all the functions for forward
> declarations as well.
> 
> Tom
> 
> 2014-05-21  Tom Tromey  <tromey@redhat.com>
> 
> 	* procfs.c (procfs_attach): Make "args" const.
> 	* 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.

I tried a few different ways to see if we may have missed any other
files, and came up empty, so hopefully you nailed them all.

For this sort of search, it would be helpful to have a convention
that we either use var->method to set the field, or else put the name
of the field in comment next to the assignment like we do with
the language vector. That really makes it easier to find them;
otherwise, one has to use code analyzers, which sometimes don't
work when you can't compile the file first.

Patch looks good to me!
Tom Tromey May 21, 2014, 9:35 p.m. UTC | #2 
Joel> For this sort of search, it would be helpful to have a convention
Joel> that we either use var->method to set the field, or else put the name
Joel> of the field in comment next to the assignment like we do with
Joel> the language vector. That really makes it easier to find them;
Joel> otherwise, one has to use code analyzers, which sometimes don't
Joel> work when you can't compile the file first.

Yeah.  C99 designated initializers are really nice for this reason.

Tom
Joel Brobecker May 21, 2014, 10:05 p.m. UTC | #3 
> Yeah.  C99 designated initializers are really nice for this reason.

I searched the GDB wiki about C99, and there were no hits. I am
wondering if we should be starting a list of C99 features we think
would be worthwhile to allow. This one would definitely be on my list!

Perhaps we can also list some of the issues that would prevent us
from adopting a subset of C99 (Eg: fear if missing checks against
disallowed features)?

(just thinking aloud)
Tom Tromey May 21, 2014, 10:55 p.m. UTC | #4 
Joel> I searched the GDB wiki about C99, and there were no hits. I am
Joel> wondering if we should be starting a list of C99 features we think
Joel> would be worthwhile to allow. This one would definitely be on my list!

I like designated init, "//" comments, varargs macros, "for (int i = ...",
_Bool, and even declaring variables at point of use (but I know others
dislike this one).

I see C99 as a convenience upgrade.  None of those things will markedly
improve gdb's quality, they may just make the hacking marginally nicer.
For me of course C99 is the runner-up choice ...

Joel> Perhaps we can also list some of the issues that would prevent us
Joel> from adopting a subset of C99 (Eg: fear if missing checks against
Joel> disallowed features)?

IIRC there was some concern about library issues.
Or maybe that GCC doesn't implement all the IEEE additions?
I don't really remember now.  It's in the list archives.

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/procfs.c b/gdb/procfs.c
index 80b0a6a..90c53e3 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -109,7 +109,7 @@ 
 
 /* This module defines the GDB target vector and its methods.  */
 
-static void procfs_attach (struct target_ops *, char *, int);
+static void procfs_attach (struct target_ops *, const char *, int);
 static void procfs_detach (struct target_ops *, const char *, int);
 static void procfs_resume (struct target_ops *,
 			   ptid_t, int, enum gdb_signal);
@@ -3038,7 +3038,7 @@  procfs_debug_inferior (procinfo *pi)
 }
 
 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;