public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] constify to_attach
@ 2014-05-21 18:21 Tom Tromey
  2014-05-21 18:58 ` Tom Tromey
  2014-05-21 19:45 ` Pedro Alves
  0 siblings, 2 replies; 8+ messages in thread
From: Tom Tromey @ 2014-05-21 18:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

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(-)

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);
 \f
 /* 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;
-- 
1.9.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] constify to_attach
  2014-05-21 18:21 [PATCH] constify to_attach Tom Tromey
@ 2014-05-21 18:58 ` Tom Tromey
  2014-05-21 21:22   ` Joel Brobecker
  2014-05-21 19:45 ` Pedro Alves
  1 sibling, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2014-05-21 18:58 UTC (permalink / raw)
  To: gdb-patches

>>>>> "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.

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);
 \f
 /* 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;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] constify to_attach
  2014-05-21 18:21 [PATCH] constify to_attach Tom Tromey
  2014-05-21 18:58 ` Tom Tromey
@ 2014-05-21 19:45 ` Pedro Alves
  2014-06-04 17:09   ` Tom Tromey
  1 sibling, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2014-05-21 19:45 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

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.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] constify to_attach
  2014-05-21 18:58 ` Tom Tromey
@ 2014-05-21 21:22   ` Joel Brobecker
  2014-05-21 21:36     ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Joel Brobecker @ 2014-05-21 21:22 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> 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!
-- 
Joel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] constify to_attach
  2014-05-21 21:22   ` Joel Brobecker
@ 2014-05-21 21:36     ` Tom Tromey
  2014-05-21 22:05       ` Joel Brobecker
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2014-05-21 21:36 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] constify to_attach
  2014-05-21 21:36     ` Tom Tromey
@ 2014-05-21 22:05       ` Joel Brobecker
  2014-05-21 22:55         ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Joel Brobecker @ 2014-05-21 22:05 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> 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)

-- 
Joel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] constify to_attach
  2014-05-21 22:05       ` Joel Brobecker
@ 2014-05-21 22:55         ` Tom Tromey
  0 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey @ 2014-05-21 22:55 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] constify to_attach
  2014-05-21 19:45 ` Pedro Alves
@ 2014-06-04 17:09   ` Tom Tromey
  0 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey @ 2014-06-04 17:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-06-04 17:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-21 18:21 [PATCH] constify to_attach Tom Tromey
2014-05-21 18:58 ` Tom Tromey
2014-05-21 21:22   ` Joel Brobecker
2014-05-21 21:36     ` Tom Tromey
2014-05-21 22:05       ` Joel Brobecker
2014-05-21 22:55         ` Tom Tromey
2014-05-21 19:45 ` Pedro Alves
2014-06-04 17:09   ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).