[PATCH c++ 01/12] Introduce ax_raw_byte and use it

[PATCH c++ 01/12] Introduce ax_raw_byte and use it

[Simon Marchi]

This patch was taken directly from Pedro's branch.

ax_simple is used to append an agent expression operator to an agent
expression string.  Therefore, it takes an enum agent_op as input.
There is an instance where it's called to append a raw byte, unrelated
to the enum.  It makes the build fail in C++ mode.

This patch introduces ax_raw_byte for that purpose and uses it.

gdb/ChangeLog:

	* ax.h (ax_raw_byte): New declaration.
	* ax-general.c (ax_raw_byte): New function.
	(ax_simple): Use ax_raw_byte.
	* ax-gdb.c (gen_printf): Likewise.
---
 gdb/ax-gdb.c     |  2 +-
 gdb/ax-general.c | 11 +++++++++--
 gdb/ax.h         |  3 +++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index 817fa53..7091a4a 100644
--- a/gdb/ax-gdb.c
+++ b/gdb/ax-gdb.c
@@ -2564,7 +2564,7 @@ gen_printf (CORE_ADDR scope, struct gdbarch *gdbarch,
 
   /* Issue the printf bytecode proper.  */
   ax_simple (ax, aop_printf);
-  ax_simple (ax, nargs);
+  ax_raw_byte (ax, nargs);
   ax_string (ax, format, fmtlen);
 
   /* And terminate.  */
diff --git a/gdb/ax-general.c b/gdb/ax-general.c
index e5dc240..5c8a25b 100644
--- a/gdb/ax-general.c
+++ b/gdb/ax-general.c
@@ -133,13 +133,20 @@ read_const (struct agent_expr *x, int o, int n)
   return accum;
 }
 
+/* See ax.h.  */
+
+void
+ax_raw_byte (struct agent_expr *x, gdb_byte byte)
+{
+  grow_expr (x, 1);
+  x->buf[x->len++] = byte;
+}
 
 /* Append a simple operator OP to EXPR.  */
 void
 ax_simple (struct agent_expr *x, enum agent_op op)
 {
-  grow_expr (x, 1);
-  x->buf[x->len++] = op;
+  ax_raw_byte (x, op);
 }
 
 /* Append a pick operator to EXPR.  DEPTH is the stack item to pick,
diff --git a/gdb/ax.h b/gdb/ax.h
index eaa72dd..1714bb4 100644
--- a/gdb/ax.h
+++ b/gdb/ax.h
@@ -190,6 +190,9 @@ extern struct agent_expr *new_agent_expr (struct gdbarch *, CORE_ADDR);
 extern void free_agent_expr (struct agent_expr *);
 extern struct cleanup *make_cleanup_free_agent_expr (struct agent_expr *);
 
+/* Append a raw byte to EXPR.  */
+extern void ax_raw_byte (struct agent_expr *expr, gdb_byte byte);
+
 /* Append a simple operator OP to EXPR.  */
 extern void ax_simple (struct agent_expr *EXPR, enum agent_op OP);
 
-- 
2.6.2

[PATCH c++ 03/12] ctf_xfer_partial: Return TARGET_XFER_E_IO instead of -1 on error

[Simon Marchi]

Use the enum value instead of the corresponding int value.

gdb/ChangeLog:

	* ctf.c (ctf_xfer_partial): Return TARGET_XFER_E_IO instead of
	-1 on error.
---
 gdb/ctf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/ctf.c b/gdb/ctf.c
index cb0d707..b8d7285 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -1279,7 +1279,7 @@ ctf_xfer_partial (struct target_ops *ops, enum target_object object,
 {
   /* We're only doing regular memory for now.  */
   if (object != TARGET_OBJECT_MEMORY)
-    return -1;
+    return TARGET_XFER_E_IO;
 
   if (readbuf == NULL)
     error (_("ctf_xfer_partial: trace file is read-only"));
-- 
2.6.2

Re: [PATCH c++ 03/12] ctf_xfer_partial: Return TARGET_XFER_E_IO instead of -1 on error

[Simon Marchi]

I pushed this one, I consider it obvious enough.

[PATCH c++ 06/12] Fix constness problem in ioscm_make_gdb_stdio_port

[Simon Marchi]

ioscm_make_gdb_stdio_port passes const char pointers (literal strings) to
scm_mode_bits, which takes a non-const char pointer.  Ideally, we would
change scm_mode_bits to take a const char pointer, but it's not part of
an API we control.

Instead, it's easy enough to build the string to pass to scm_mode_bits in
a (non-const) char array and pass that.

gdb/ChangeLog:

	* guile/scm-ports.c (ioscm_make_gdb_stdio_port): Pass non-const
	char pointer to scm_mode_bits.
---
 gdb/guile/scm-ports.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 925f3b2..49e4fd6 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -357,29 +357,38 @@ ioscm_init_stdio_buffers (SCM port, long mode_bits)
 static SCM
 ioscm_make_gdb_stdio_port (int fd)
 {
-  int is_a_tty = isatty (fd);
   const char *name;
   long mode_bits;
   SCM port;
+  char buf[3];
+
+  memset (buf, 0, sizeof (buf));
 
   switch (fd)
     {
     case 0:
       name = input_port_name;
-      mode_bits = scm_mode_bits (is_a_tty ? "r0" : "r");
+      buf[0] = 'r';
       break;
     case 1:
       name = output_port_name;
-      mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
+      buf[0] = 'w';
       break;
     case 2:
       name = error_port_name;
-      mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
+      buf[0] = 'w';
       break;
     default:
       gdb_assert_not_reached ("bad stdio file descriptor");
     }
 
+  if (isatty (fd))
+    {
+      buf[1] = '0';
+    }
+
+  mode_bits = scm_mode_bits (buf);
+
   port = ioscm_open_port (stdio_port_desc, mode_bits);
 
   scm_set_port_filename_x (port, gdbscm_scm_from_c_string (name));
-- 
2.6.2

Re: [PATCH c++ 06/12] Fix constness problem in ioscm_make_gdb_stdio_port

[Doug Evans]

Simon Marchi <simon.marchi@polymtl.ca> writes:
> ioscm_make_gdb_stdio_port passes const char pointers (literal strings) to
> scm_mode_bits, which takes a non-const char pointer.  Ideally, we would
> change scm_mode_bits to take a const char pointer, but it's not part of
> an API we control.
>
> Instead, it's easy enough to build the string to pass to scm_mode_bits in
> a (non-const) char array and pass that.
>
> gdb/ChangeLog:
>
> 	* guile/scm-ports.c (ioscm_make_gdb_stdio_port): Pass non-const
> 	char pointer to scm_mode_bits.
> ---
>  gdb/guile/scm-ports.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
> index 925f3b2..49e4fd6 100644
> --- a/gdb/guile/scm-ports.c
> +++ b/gdb/guile/scm-ports.c
> @@ -357,29 +357,38 @@ ioscm_init_stdio_buffers (SCM port, long mode_bits)
>  static SCM
>  ioscm_make_gdb_stdio_port (int fd)
>  {
> -  int is_a_tty = isatty (fd);
>    const char *name;
>    long mode_bits;
>    SCM port;
> +  char buf[3];
> +
> +  memset (buf, 0, sizeof (buf));
>  
>    switch (fd)
>      {
>      case 0:
>        name = input_port_name;
> -      mode_bits = scm_mode_bits (is_a_tty ? "r0" : "r");
> +      buf[0] = 'r';
>        break;
>      case 1:
>        name = output_port_name;
> -      mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
> +      buf[0] = 'w';
>        break;
>      case 2:
>        name = error_port_name;
> -      mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
> +      buf[0] = 'w';
>        break;
>      default:
>        gdb_assert_not_reached ("bad stdio file descriptor");
>      }
>  
> +  if (isatty (fd))
> +    {
> +      buf[1] = '0';
> +    }

Community rules say to not use braces here.
Ok with that change.

> +
> +  mode_bits = scm_mode_bits (buf);
> +
>    port = ioscm_open_port (stdio_port_desc, mode_bits);
>  
>    scm_set_port_filename_x (port, gdbscm_scm_from_c_string (name));

Re: [PATCH c++ 06/12] Fix constness problem in ioscm_make_gdb_stdio_port

[Pedro Alves]

On 10/26/2015 03:46 AM, Simon Marchi wrote:
> ioscm_make_gdb_stdio_port passes const char pointers (literal strings) to
> scm_mode_bits, which takes a non-const char pointer.  Ideally, we would
> change scm_mode_bits to take a const char pointer, but it's not part of
> an API we control.
> 
> Instead, it's easy enough to build the string to pass to scm_mode_bits in
> a (non-const) char array and pass that.
> 
> gdb/ChangeLog:
> 
> 	* guile/scm-ports.c (ioscm_make_gdb_stdio_port): Pass non-const
> 	char pointer to scm_mode_bits.
> ---
>  gdb/guile/scm-ports.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
> index 925f3b2..49e4fd6 100644
> --- a/gdb/guile/scm-ports.c
> +++ b/gdb/guile/scm-ports.c
> @@ -357,29 +357,38 @@ ioscm_init_stdio_buffers (SCM port, long mode_bits)
>  static SCM
>  ioscm_make_gdb_stdio_port (int fd)
>  {
> -  int is_a_tty = isatty (fd);
>    const char *name;
>    long mode_bits;
>    SCM port;
> +  char buf[3];
> +
> +  memset (buf, 0, sizeof (buf));
>  
>    switch (fd)
>      {
>      case 0:
>        name = input_port_name;
> -      mode_bits = scm_mode_bits (is_a_tty ? "r0" : "r");
> +      buf[0] = 'r';
>        break;
>      case 1:
>        name = output_port_name;
> -      mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
> +      buf[0] = 'w';
>        break;
>      case 2:
>        name = error_port_name;
> -      mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
> +      buf[0] = 'w';
>        break;
>      default:
>        gdb_assert_not_reached ("bad stdio file descriptor");
>      }
>  
> +  if (isatty (fd))
> +    {
> +      buf[1] = '0';
> +    }
> +
> +  mode_bits = scm_mode_bits (buf);
> +
>    port = ioscm_open_port (stdio_port_desc, mode_bits);
>  
>    scm_set_port_filename_x (port, gdbscm_scm_from_c_string (name));
> 

Hmmm, I have the below on my branch, which seems clearer/straightforward
to me.  I don't think the cast in the scm_mode_bits call is problem.

It's a deficiency of the scm_mode_bits api that it doesn't take a const
pointer, and we actually already have this in ioscm_parse_mode_bits:

  /* Kinda awkward to convert the mode from SCM -> string only to have Guile
     convert it back to SCM, but that's the API we have to work with.  */
  mode_bits = scm_mode_bits ((char *) mode);

WDYT?

From f1038881f2b79d26968f2552b86612324fdf72c3 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 22 Oct 2015 11:43:57 +0100
Subject: [PATCH] guile: 'const char *' -> 'char *' casts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

/home/pedro/gdb/mygit/src/gdb/guile/scm-ports.c: In function ‘scm_unused_struct* ioscm_make_gdb_stdio_port(int)’:
/home/pedro/gdb/mygit/src/gdb/guile/scm-ports.c:369:55: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
       mode_bits = scm_mode_bits (is_a_tty ? "r0" : "r");
                                                       ^
In file included from /usr/include/guile/2.0/libguile/fports.h:28:0,
                 from /usr/include/guile/2.0/libguile.h:55,
                 from /home/pedro/gdb/mygit/src/gdb/guile/guile-internal.h:29,
                 from /home/pedro/gdb/mygit/src/gdb/guile/scm-ports.c:28:
/usr/include/guile/2.0/libguile/ports.h:281:14: error:   initializing argument 1 of ‘long int scm_mode_bits(char*)’ [-fpermissive]
 SCM_API long scm_mode_bits (char *modes);
              ^
/home/pedro/gdb/mygit/src/gdb/guile/scm-ports.c:373:55: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
       mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
                                                       ^
In file included from /usr/include/guile/2.0/libguile/fports.h:28:0,
                 from /usr/include/guile/2.0/libguile.h:55,
                 from /home/pedro/gdb/mygit/src/gdb/guile/guile-internal.h:29,
                 from /home/pedro/gdb/mygit/src/gdb/guile/scm-ports.c:28:
/usr/include/guile/2.0/libguile/ports.h:281:14: error:   initializing argument 1 of ‘long int scm_mode_bits(char*)’ [-fpermissive]
 SCM_API long scm_mode_bits (char *modes);
              ^
/home/pedro/gdb/mygit/src/gdb/guile/scm-ports.c:377:55: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
       mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
                                                       ^
In file included from /usr/include/guile/2.0/libguile/fports.h:28:0,
                 from /usr/include/guile/2.0/libguile.h:55,
                 from /home/pedro/gdb/mygit/src/gdb/guile/guile-internal.h:29,
                 from /home/pedro/gdb/mygit/src/gdb/guile/scm-ports.c:28:
/usr/include/guile/2.0/libguile/ports.h:281:14: error:   initializing argument 1 of ‘long int scm_mode_bits(char*)’ [-fpermissive]
 SCM_API long scm_mode_bits (char *modes);
---
 gdb/guile/scm-ports.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 90bdb39..d8ee70e 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -359,6 +359,7 @@ ioscm_make_gdb_stdio_port (int fd)
 {
   int is_a_tty = isatty (fd);
   const char *name;
+  const char *mode_str;
   long mode_bits;
   SCM port;
 
@@ -366,20 +367,21 @@ ioscm_make_gdb_stdio_port (int fd)
     {
     case 0:
       name = input_port_name;
-      mode_bits = scm_mode_bits (is_a_tty ? "r0" : "r");
+      mode_str = is_a_tty ? "r0" : "r";
       break;
     case 1:
       name = output_port_name;
-      mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
+      mode_str = is_a_tty ? "w0" : "w";
       break;
     case 2:
       name = error_port_name;
-      mode_bits = scm_mode_bits (is_a_tty ? "w0" : "w");
+      mode_str = is_a_tty ? "w0" : "w";
       break;
     default:
       gdb_assert_not_reached ("bad stdio file descriptor");
     }
 
+  mode_bits = scm_mode_bits ((char *) mode_str);
   port = ioscm_open_port (stdio_port_desc, mode_bits);
 
   scm_set_port_filename_x (port, gdbscm_scm_from_c_string (name));
-- 
1.9.3

Re: [PATCH c++ 06/12] Fix constness problem in ioscm_make_gdb_stdio_port

[Simon Marchi]

On 26 October 2015 at 12:09, Pedro Alves <palves@redhat.com> wrote:
> WDYT?

Sounds good to me.  I wanted to avoid passing a string literal to
something that wasn't explicitely const (since if it tries to modify
the string, the program just crashes).  But if we are sure that
scm_mode_bits does not do that, we can keep the simpler code.

Here's a ready to push patch (not guaranteed to be well-formatted,
because gmail).


From ff3fa55ca1cf6bd23de87eb26687ad48fb2b3806 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Mon, 26 Oct 2015 13:26:51 -0400
Subject: [PATCH] guile: Simplify ioscm_make_gdb_stdio_port

As pointed out by Pedro, it's clearer to do it this way.  We can trust
that scm_mode_bits won't try to modify our string, even though it takes
a non-const char *.

gdb/ChangeLog:

    * guile/scm-ports.c (ioscm_make_gdb_stdio_port): Do not pass a
    local char array to scm_mode_bits, use a cast instead.
---
 gdb/guile/scm-ports.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 5d529b3..ffaa3fd 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -357,36 +357,31 @@ ioscm_init_stdio_buffers (SCM port, long mode_bits)
 static SCM
 ioscm_make_gdb_stdio_port (int fd)
 {
+  int is_a_tty = isatty (fd);
   const char *name;
+  const char *mode_str;
   long mode_bits;
   SCM port;
-  char buf[3];
-
-  memset (buf, 0, sizeof (buf));

   switch (fd)
     {
     case 0:
       name = input_port_name;
-      buf[0] = 'r';
+      mode_str = is_a_tty ? "r0" : "r";
       break;
     case 1:
       name = output_port_name;
-      buf[0] = 'w';
+      mode_str = is_a_tty ? "w0" : "w";
       break;
     case 2:
       name = error_port_name;
-      buf[0] = 'w';
+      mode_str = is_a_tty ? "w0" : "w";
       break;
     default:
       gdb_assert_not_reached ("bad stdio file descriptor");
     }

-  if (isatty (fd))
-    buf[1] = '0';
-
-  mode_bits = scm_mode_bits (buf);
-
+  mode_bits = scm_mode_bits ((char *) mode_str);
   port = ioscm_open_port (stdio_port_desc, mode_bits);

   scm_set_port_filename_x (port, gdbscm_scm_from_c_string (name));
-- 
2.5.1

Re: [PATCH c++ 06/12] Fix constness problem in ioscm_make_gdb_stdio_port

[Pedro Alves]

On 10/26/2015 05:27 PM, Simon Marchi wrote:
> On 26 October 2015 at 12:09, Pedro Alves <palves@redhat.com> wrote:
>> WDYT?
> 
> Sounds good to me.  I wanted to avoid passing a string literal to
> something that wasn't explicitely const (since if it tries to modify
> the string, the program just crashes).  But if we are sure that
> scm_mode_bits does not do that, we can keep the simpler code.
> 
> Here's a ready to push patch (not guaranteed to be well-formatted,
> because gmail).
> 
> 
> From ff3fa55ca1cf6bd23de87eb26687ad48fb2b3806 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Mon, 26 Oct 2015 13:26:51 -0400
> Subject: [PATCH] guile: Simplify ioscm_make_gdb_stdio_port
> 
> As pointed out by Pedro, it's clearer to do it this way.  We can trust
> that scm_mode_bits won't try to modify our string, even though it takes
> a non-const char *.

LGTM, though it's Doug's call.

Thanks,
Pedro Alves

Re: [PATCH c++ 06/12] Fix constness problem in ioscm_make_gdb_stdio_port

[Doug Evans]

On Mon, Oct 26, 2015 at 10:39 AM, Pedro Alves <palves@redhat.com> wrote:
> On 10/26/2015 05:27 PM, Simon Marchi wrote:
>> On 26 October 2015 at 12:09, Pedro Alves <palves@redhat.com> wrote:
>>> WDYT?
>>
>> Sounds good to me.  I wanted to avoid passing a string literal to
>> something that wasn't explicitely const (since if it tries to modify
>> the string, the program just crashes).  But if we are sure that
>> scm_mode_bits does not do that, we can keep the simpler code.
>>
>> Here's a ready to push patch (not guaranteed to be well-formatted,
>> because gmail).
>>
>>
>> From ff3fa55ca1cf6bd23de87eb26687ad48fb2b3806 Mon Sep 17 00:00:00 2001
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> Date: Mon, 26 Oct 2015 13:26:51 -0400
>> Subject: [PATCH] guile: Simplify ioscm_make_gdb_stdio_port
>>
>> As pointed out by Pedro, it's clearer to do it this way.  We can trust
>> that scm_mode_bits won't try to modify our string, even though it takes
>> a non-const char *.
>
> LGTM, though it's Doug's call.

Fine by me.

Re: [PATCH c++ 06/12] Fix constness problem in ioscm_make_gdb_stdio_port

[Simon Marchi]

On 26 October 2015 at 14:36, Doug Evans <xdje42@gmail.com> wrote:
> Fine by me.

Thanks, pushed.

[PATCH c++ 08/12] scm-symbol.c: Add (domain_enum) casts

[Simon Marchi]

We currently pass integers as domain_enums to lookup_symbol.  The
most obvious fix is to add casts there.

I first thought of changing the type of the domain variables to
domain_enum.  However, because we pass a pointer to them to
gdbscm_parse_function_args, which expects them to be integers (because
of the format string), I don't think it would be correct.  If the enum
does not have the same size as an int, gdbscm_parse_function_args could
write past the memory of domain, overwriting something else on the
stack.

gdb/ChangeLog:

	* guile/scm-symbol.c (gdbscm_lookup_global_symbol): Add
	domain_enum cast.
	(gdbscm_lookup_symbol): Likewise.
---
 gdb/guile/scm-symbol.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/guile/scm-symbol.c b/gdb/guile/scm-symbol.c
index 81e4d50..10400be 100644
--- a/gdb/guile/scm-symbol.c
+++ b/gdb/guile/scm-symbol.c
@@ -622,7 +622,8 @@ gdbscm_lookup_symbol (SCM name_scm, SCM rest)
 
   TRY
     {
-      symbol = lookup_symbol (name, block, domain, &is_a_field_of_this).symbol;
+      symbol = lookup_symbol (name, block, (domain_enum) domain,
+			      &is_a_field_of_this).symbol;
     }
   CATCH (ex, RETURN_MASK_ALL)
     {
@@ -662,7 +663,7 @@ gdbscm_lookup_global_symbol (SCM name_scm, SCM rest)
 
   TRY
     {
-      symbol = lookup_global_symbol (name, NULL, domain).symbol;
+      symbol = lookup_global_symbol (name, NULL, (domain_enum) domain).symbol;
     }
   CATCH (ex, RETURN_MASK_ALL)
     {
-- 
2.6.2

Re: [PATCH c++ 08/12] scm-symbol.c: Add (domain_enum) casts

[Doug Evans]

Simon Marchi <simon.marchi@polymtl.ca> writes:
> We currently pass integers as domain_enums to lookup_symbol.  The
> most obvious fix is to add casts there.
>
> I first thought of changing the type of the domain variables to
> domain_enum.  However, because we pass a pointer to them to
> gdbscm_parse_function_args, which expects them to be integers (because
> of the format string), I don't think it would be correct.  If the enum
> does not have the same size as an int, gdbscm_parse_function_args could
> write past the memory of domain, overwriting something else on the
> stack.
>
> gdb/ChangeLog:
>
> 	* guile/scm-symbol.c (gdbscm_lookup_global_symbol): Add
> 	domain_enum cast.
> 	(gdbscm_lookup_symbol): Likewise.
> ---
>  gdb/guile/scm-symbol.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/guile/scm-symbol.c b/gdb/guile/scm-symbol.c
> index 81e4d50..10400be 100644
> --- a/gdb/guile/scm-symbol.c
> +++ b/gdb/guile/scm-symbol.c
> @@ -622,7 +622,8 @@ gdbscm_lookup_symbol (SCM name_scm, SCM rest)
>  
>    TRY
>      {
> -      symbol = lookup_symbol (name, block, domain, &is_a_field_of_this).symbol;
> +      symbol = lookup_symbol (name, block, (domain_enum) domain,
> +			      &is_a_field_of_this).symbol;
>      }
>    CATCH (ex, RETURN_MASK_ALL)
>      {
> @@ -662,7 +663,7 @@ gdbscm_lookup_global_symbol (SCM name_scm, SCM rest)
>  
>    TRY
>      {
> -      symbol = lookup_global_symbol (name, NULL, domain).symbol;
> +      symbol = lookup_global_symbol (name, NULL, (domain_enum) domain).symbol;
>      }
>    CATCH (ex, RETURN_MASK_ALL)
>      {

LGTM

[PATCH c++ 05/12] guile: Constify gdbscm_with_guile return value

[Simon Marchi]

Initially fixess:

/home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c: In function ‘void* gdbscm_disasm_read_memory_worker(void*)’:
/home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c:93:12: error: invalid conversion from ‘const void*’ to ‘void*’ [-fpermissive]
     return "seek error";

This makes const the whole path that leads to the return of
gdbscm_with_guile.

gdb/ChangeLog:

	* guile/guile-internal.h (gdbscm_with_guile): Constify function
	pointer return value and self return value.
	* guile/scm-safe-call.c (gdbscm_with_guile): Likewise.
	(struct c_data) <func>: Constify return value.
	(struct c_data) <result>: Constify.
	(scscm_eval_scheme_string): Constify return value.
	(gdbscm_safe_eval_string): Constify status variable.
	(scscm_source_scheme_script): Constify return value.
	(gdbscm_safe_source_script): Constify status variable.
	* guile/scm-disasm.c (gdbscm_disasm_read_memory_worker):
	Constify returrn value.
	(gdbscm_disasm_read_memory): Constify status variable.
i
---
 gdb/guile/guile-internal.h |  2 +-
 gdb/guile/scm-disasm.c     |  4 ++--
 gdb/guile/scm-safe-call.c  | 20 ++++++++++----------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index 017309a..0af01d2 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -384,7 +384,7 @@ extern void gdbscm_memory_error (const char *subr, const char *msg, SCM args)
 
 /* scm-safe-call.c */
 
-extern void *gdbscm_with_guile (void *(*func) (void *), void *data);
+extern const void *gdbscm_with_guile (const void *(*func) (void *), void *data);
 
 extern SCM gdbscm_call_guile (SCM (*func) (void *), void *data,
 			      excp_matcher_func *ok_excps);
diff --git a/gdb/guile/scm-disasm.c b/gdb/guile/scm-disasm.c
index d1572c7..63889c1 100644
--- a/gdb/guile/scm-disasm.c
+++ b/gdb/guile/scm-disasm.c
@@ -76,7 +76,7 @@ dascm_make_insn (CORE_ADDR pc, const char *assembly, int insn_len)
    Scheme port.  Called via gdbscm_call_guile.
    The result is a statically allocated error message or NULL if success.  */
 
-static void *
+static const void *
 gdbscm_disasm_read_memory_worker (void *datap)
 {
   struct gdbscm_disasm_read_data *data
@@ -109,7 +109,7 @@ gdbscm_disasm_read_memory (bfd_vma memaddr, bfd_byte *myaddr,
 			   struct disassemble_info *dinfo)
 {
   struct gdbscm_disasm_read_data data;
-  void *status;
+  const void *status;
 
   data.memaddr = memaddr;
   data.myaddr = myaddr;
diff --git a/gdb/guile/scm-safe-call.c b/gdb/guile/scm-safe-call.c
index 62aec0f..85a2a5b 100644
--- a/gdb/guile/scm-safe-call.c
+++ b/gdb/guile/scm-safe-call.c
@@ -28,10 +28,10 @@
 
 struct c_data
 {
-  void *(*func) (void *);
+  const void *(*func) (void *);
   void *data;
   /* An error message or NULL for success.  */
-  void *result;
+  const void *result;
 };
 
 /* Struct to marshall args through gdbscm_with_catch.  */
@@ -167,8 +167,8 @@ gdbscm_with_catch (void *data)
    The result if NULL if no exception occurred, otherwise it is a statically
    allocated error message (caller must *not* free).  */
 
-void *
-gdbscm_with_guile (void *(*func) (void *), void *data)
+const void *
+gdbscm_with_guile (const void *(*func) (void *), void *data)
 {
   struct c_data c_data;
   struct with_catch_data catch_data;
@@ -369,7 +369,7 @@ struct eval_scheme_string_data
 /* Wrapper to eval a C string in the Guile interpreter.
    This is passed to gdbscm_with_guile.  */
 
-static void *
+static const void *
 scscm_eval_scheme_string (void *datap)
 {
   struct eval_scheme_string_data *data
@@ -398,12 +398,12 @@ char *
 gdbscm_safe_eval_string (const char *string, int display_result)
 {
   struct eval_scheme_string_data data = { string, display_result };
-  void *result;
+  const void *result;
 
   result = gdbscm_with_guile (scscm_eval_scheme_string, (void *) &data);
 
   if (result != NULL)
-    return xstrdup ((char *) result);
+    return xstrdup ((const char *) result);
   return NULL;
 }
 \f
@@ -411,7 +411,7 @@ gdbscm_safe_eval_string (const char *string, int display_result)
 
 /* Helper function for gdbscm_safe_source_scheme_script.  */
 
-static void *
+static const void *
 scscm_source_scheme_script (void *data)
 {
   const char *filename = (const char *) data;
@@ -439,7 +439,7 @@ gdbscm_safe_source_script (const char *filename)
      by default.  This function is invoked by the "source" GDB command which
      already has its own path search support.  */
   char *abs_filename = NULL;
-  void *result;
+  const void *result;
 
   if (!IS_ABSOLUTE_PATH (filename))
     {
@@ -452,7 +452,7 @@ gdbscm_safe_source_script (const char *filename)
 
   xfree (abs_filename);
   if (result != NULL)
-    return xstrdup ((char *) result);
+    return xstrdup ((const char *) result);
   return NULL;
 }
 \f
-- 
2.6.2

Re: [PATCH c++ 05/12] guile: Constify gdbscm_with_guile return value

[Doug Evans]

Simon Marchi <simon.marchi@polymtl.ca> writes:
> Initially fixess:
>
> /home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c: In function ‘void* gdbscm_disasm_read_memory_worker(void*)’:
> /home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c:93:12: error: invalid conversion from ‘const void*’ to ‘void*’ [-fpermissive]
>      return "seek error";
>
> This makes const the whole path that leads to the return of
> gdbscm_with_guile.
>
> gdb/ChangeLog:
>
> 	* guile/guile-internal.h (gdbscm_with_guile): Constify function
> 	pointer return value and self return value.
> 	* guile/scm-safe-call.c (gdbscm_with_guile): Likewise.
> 	(struct c_data) <func>: Constify return value.
> 	(struct c_data) <result>: Constify.
> 	(scscm_eval_scheme_string): Constify return value.
> 	(gdbscm_safe_eval_string): Constify status variable.
> 	(scscm_source_scheme_script): Constify return value.
> 	(gdbscm_safe_source_script): Constify status variable.
> 	* guile/scm-disasm.c (gdbscm_disasm_read_memory_worker):
> 	Constify returrn value.
> 	(gdbscm_disasm_read_memory): Constify status variable.

Hi.

How about instead having gdbscm_with_guile return a const char *.
That should, for example, remove the need for any cast here in
gdbscm_safe_source_script (and presumably elsewhere):

  if (result != NULL)
    return xstrdup ((char *) result);

>
> diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
> index 017309a..0af01d2 100644
> --- a/gdb/guile/guile-internal.h
> +++ b/gdb/guile/guile-internal.h
> @@ -384,7 +384,7 @@ extern void gdbscm_memory_error (const char *subr, const char *msg, SCM args)
>  
>  /* scm-safe-call.c */
>  
> -extern void *gdbscm_with_guile (void *(*func) (void *), void *data);
> +extern const void *gdbscm_with_guile (const void *(*func) (void *), void *data);
>  
>  extern SCM gdbscm_call_guile (SCM (*func) (void *), void *data,
>  			      excp_matcher_func *ok_excps);
> diff --git a/gdb/guile/scm-disasm.c b/gdb/guile/scm-disasm.c
> index d1572c7..63889c1 100644
> --- a/gdb/guile/scm-disasm.c
> +++ b/gdb/guile/scm-disasm.c
> @@ -76,7 +76,7 @@ dascm_make_insn (CORE_ADDR pc, const char *assembly, int insn_len)
>     Scheme port.  Called via gdbscm_call_guile.
>     The result is a statically allocated error message or NULL if success.  */
>  
> -static void *
> +static const void *
>  gdbscm_disasm_read_memory_worker (void *datap)
>  {
>    struct gdbscm_disasm_read_data *data
> @@ -109,7 +109,7 @@ gdbscm_disasm_read_memory (bfd_vma memaddr, bfd_byte *myaddr,
>  			   struct disassemble_info *dinfo)
>  {
>    struct gdbscm_disasm_read_data data;
> -  void *status;
> +  const void *status;
>  
>    data.memaddr = memaddr;
>    data.myaddr = myaddr;
> diff --git a/gdb/guile/scm-safe-call.c b/gdb/guile/scm-safe-call.c
> index 62aec0f..85a2a5b 100644
> --- a/gdb/guile/scm-safe-call.c
> +++ b/gdb/guile/scm-safe-call.c
> @@ -28,10 +28,10 @@
>  
>  struct c_data
>  {
> -  void *(*func) (void *);
> +  const void *(*func) (void *);
>    void *data;
>    /* An error message or NULL for success.  */
> -  void *result;
> +  const void *result;
>  };
>  
>  /* Struct to marshall args through gdbscm_with_catch.  */
> @@ -167,8 +167,8 @@ gdbscm_with_catch (void *data)
>     The result if NULL if no exception occurred, otherwise it is a statically
>     allocated error message (caller must *not* free).  */
>  
> -void *
> -gdbscm_with_guile (void *(*func) (void *), void *data)
> +const void *
> +gdbscm_with_guile (const void *(*func) (void *), void *data)
>  {
>    struct c_data c_data;
>    struct with_catch_data catch_data;
> @@ -369,7 +369,7 @@ struct eval_scheme_string_data
>  /* Wrapper to eval a C string in the Guile interpreter.
>     This is passed to gdbscm_with_guile.  */
>  
> -static void *
> +static const void *
>  scscm_eval_scheme_string (void *datap)
>  {
>    struct eval_scheme_string_data *data
> @@ -398,12 +398,12 @@ char *
>  gdbscm_safe_eval_string (const char *string, int display_result)
>  {
>    struct eval_scheme_string_data data = { string, display_result };
> -  void *result;
> +  const void *result;
>  
>    result = gdbscm_with_guile (scscm_eval_scheme_string, (void *) &data);
>  
>    if (result != NULL)
> -    return xstrdup ((char *) result);
> +    return xstrdup ((const char *) result);
>    return NULL;
>  }
>  \f
> @@ -411,7 +411,7 @@ gdbscm_safe_eval_string (const char *string, int display_result)
>  
>  /* Helper function for gdbscm_safe_source_scheme_script.  */
>  
> -static void *
> +static const void *
>  scscm_source_scheme_script (void *data)
>  {
>    const char *filename = (const char *) data;
> @@ -439,7 +439,7 @@ gdbscm_safe_source_script (const char *filename)
>       by default.  This function is invoked by the "source" GDB command which
>       already has its own path search support.  */
>    char *abs_filename = NULL;
> -  void *result;
> +  const void *result;
>  
>    if (!IS_ABSOLUTE_PATH (filename))
>      {
> @@ -452,7 +452,7 @@ gdbscm_safe_source_script (const char *filename)
>  
>    xfree (abs_filename);
>    if (result != NULL)
> -    return xstrdup ((char *) result);
> +    return xstrdup ((const char *) result);
>    return NULL;
>  }
>  \f

Re: [PATCH c++ 05/12] guile: Constify gdbscm_with_guile return value

[Simon Marchi]

On 26 October 2015 at 01:22, Doug Evans <xdje42@gmail.com> wrote:
> Hi.
>
> How about instead having gdbscm_with_guile return a const char *.
> That should, for example, remove the need for any cast here in
> gdbscm_safe_source_script (and presumably elsewhere):
>
>   if (result != NULL)


Hmmm, looking more at the issue, I see that we happen to call
gdbscm_with_guile only with functions that return const char *.  But
it doesn't mean we should make gdbscm_with_guile return const char *
ncecessarily. It (and scm_with_guile) takes a void* and returns a
void* in order to be generic, just like in the pthread_create/join
API. The caller is responsible to cast the void* to the right type.
In C++, we could always get fancy and make it a templated function to
get type-safety.

So in retrospect, I think we should leave the original return type
(non-const void*) and just add the appropriate casts.

Does that make sense?

Re: [PATCH c++ 05/12] guile: Constify gdbscm_with_guile return value

[Doug Evans]

Simon Marchi <simon.marchi@polymtl.ca> writes:
> On 26 October 2015 at 01:22, Doug Evans <xdje42@gmail.com> wrote:
>> Hi.
>>
>> How about instead having gdbscm_with_guile return a const char *.
>> That should, for example, remove the need for any cast here in
>> gdbscm_safe_source_script (and presumably elsewhere):
>>
>>   if (result != NULL)
>
>
> Hmmm, looking more at the issue, I see that we happen to call
> gdbscm_with_guile only with functions that return const char *.  But
> it doesn't mean we should make gdbscm_with_guile return const char *
> ncecessarily. It (and scm_with_guile) takes a void* and returns a
> void* in order to be generic, just like in the pthread_create/join
> API. The caller is responsible to cast the void* to the right type.
> In C++, we could always get fancy and make it a templated function to
> get type-safety.
>
> So in retrospect, I think we should leave the original return type
> (non-const void*) and just add the appropriate casts.
>
> Does that make sense?

The function comment for gdbscm_with_guile says:

/* A wrapper around scm_with_guile that prints backtraces and exceptions
   according to "set guile print-stack".
   The result if NULL if no exception occurred, otherwise it is a statically
   allocated error message (caller must *not* free).  */

If we're going to return an error message,
why make it a void * and not a char * (const as appropriate)?

The lower level guile API uses a void * because it doesn't specify what
the result is. But in this use of it we do specify what the result is.

Re: [PATCH c++ 05/12] guile: Constify gdbscm_with_guile return value

[Simon Marchi]

On 26 October 2015 at 12:21, Doug Evans <xdje42@gmail.com> wrote:
> The function comment for gdbscm_with_guile says:
>
> /* A wrapper around scm_with_guile that prints backtraces and exceptions
>    according to "set guile print-stack".
>    The result if NULL if no exception occurred, otherwise it is a statically
>    allocated error message (caller must *not* free).  */
>
> If we're going to return an error message,
> why make it a void * and not a char * (const as appropriate)?
>
> The lower level guile API uses a void * because it doesn't specify what
> the result is. But in this use of it we do specify what the result is.

It totally makes sense.  Here is the updated patch (probably garbled
by gmail, sorry about that):


From 2defdf7de52146ea7508d0dda2f0c59e7fdd42fe Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon dot marchi at polymtl dot ca>
Date: Sun, 25 Oct 2015 23:46:37 -0400
Subject: [PATCH] guile: Change return value of gdbscm_with_guile for const
 char *
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The documentation of gdbscm_with_guile says that it returns a statically
allocated string (IOW, a const char *).  We can reflect that in its
return value type, and get rid of C++ build errors.

Initially fixes:

/home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c: In function
‘void* gdbscm_disasm_read_memory_worker(void*)’:
/home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c:93:12: error:
invalid conversion from ‘const void*’ to ‘void*’ [-fpermissive]
     return "seek error";

gdb/ChangeLog:

    * guile/guile-internal.h (gdbscm_with_guile): Change return
    types to const char *.
    * guile/scm-safe-call.c (gdbscm_with_guile): Likewise.
    (struct c_data) <func>: Likewise.
    (struct c_data) <result>: Change type to const char *.
    (scscm_eval_scheme_string): Change return type to
    const char *.
    (scscm_source_scheme_script): Likewise.
    (gdbscm_safe_eval_string): Change type of result variable to
    const char * and remove cast.
    (gdbscm_safe_source_script): Likewise.
    * guile/scm-disasm.c (gdbscm_disasm_read_memory_worker):
    Change return type to const char *.
    (gdbscm_disasm_read_memory): Change type of status to
    const char *.
---
 gdb/guile/guile-internal.h |  2 +-
 gdb/guile/scm-disasm.c     |  4 ++--
 gdb/guile/scm-safe-call.c  | 20 ++++++++++----------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index 017309a..ebf40eb 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -384,7 +384,7 @@ extern void gdbscm_memory_error (const char *subr,
const char *msg, SCM args)

 /* scm-safe-call.c */

-extern void *gdbscm_with_guile (void *(*func) (void *), void *data);
+extern const char *gdbscm_with_guile (const char *(*func) (void *),
void *data);

 extern SCM gdbscm_call_guile (SCM (*func) (void *), void *data,
                   excp_matcher_func *ok_excps);
diff --git a/gdb/guile/scm-disasm.c b/gdb/guile/scm-disasm.c
index 18e1713..0cc2f84 100644
--- a/gdb/guile/scm-disasm.c
+++ b/gdb/guile/scm-disasm.c
@@ -76,7 +76,7 @@ dascm_make_insn (CORE_ADDR pc, const char *assembly,
int insn_len)
    Scheme port.  Called via gdbscm_call_guile.
    The result is a statically allocated error message or NULL if success.  */

-static void *
+static const char *
 gdbscm_disasm_read_memory_worker (void *datap)
 {
   struct gdbscm_disasm_read_data *data
@@ -109,7 +109,7 @@ gdbscm_disasm_read_memory (bfd_vma memaddr,
bfd_byte *myaddr,
                struct disassemble_info *dinfo)
 {
   struct gdbscm_disasm_read_data data;
-  void *status;
+  const char *status;

   data.memaddr = memaddr;
   data.myaddr = myaddr;
diff --git a/gdb/guile/scm-safe-call.c b/gdb/guile/scm-safe-call.c
index 62aec0f..4e24134 100644
--- a/gdb/guile/scm-safe-call.c
+++ b/gdb/guile/scm-safe-call.c
@@ -28,10 +28,10 @@

 struct c_data
 {
-  void *(*func) (void *);
+  const char *(*func) (void *);
   void *data;
   /* An error message or NULL for success.  */
-  void *result;
+  const char *result;
 };

 /* Struct to marshall args through gdbscm_with_catch.  */
@@ -167,8 +167,8 @@ gdbscm_with_catch (void *data)
    The result if NULL if no exception occurred, otherwise it is a statically
    allocated error message (caller must *not* free).  */

-void *
-gdbscm_with_guile (void *(*func) (void *), void *data)
+const char *
+gdbscm_with_guile (const char *(*func) (void *), void *data)
 {
   struct c_data c_data;
   struct with_catch_data catch_data;
@@ -369,7 +369,7 @@ struct eval_scheme_string_data
 /* Wrapper to eval a C string in the Guile interpreter.
    This is passed to gdbscm_with_guile.  */

-static void *
+static const char *
 scscm_eval_scheme_string (void *datap)
 {
   struct eval_scheme_string_data *data
@@ -398,12 +398,12 @@ char *
 gdbscm_safe_eval_string (const char *string, int display_result)
 {
   struct eval_scheme_string_data data = { string, display_result };
-  void *result;
+  const char *result;

   result = gdbscm_with_guile (scscm_eval_scheme_string, (void *) &data);

   if (result != NULL)
-    return xstrdup ((char *) result);
+    return xstrdup (result);
   return NULL;
 }

@@ -411,7 +411,7 @@ gdbscm_safe_eval_string (const char *string, int
display_result)

 /* Helper function for gdbscm_safe_source_scheme_script.  */

-static void *
+static const char *
 scscm_source_scheme_script (void *data)
 {
   const char *filename = (const char *) data;
@@ -439,7 +439,7 @@ gdbscm_safe_source_script (const char *filename)
      by default.  This function is invoked by the "source" GDB command which
      already has its own path search support.  */
   char *abs_filename = NULL;
-  void *result;
+  const char *result;

   if (!IS_ABSOLUTE_PATH (filename))
     {
@@ -452,7 +452,7 @@ gdbscm_safe_source_script (const char *filename)

   xfree (abs_filename);
   if (result != NULL)
-    return xstrdup ((char *) result);
+    return xstrdup (result);
   return NULL;
 }

-- 
2.6.1

Re: [PATCH c++ 05/12] guile: Constify gdbscm_with_guile return value

[Doug Evans]

Simon Marchi <simon.marchi@polymtl.ca> writes:
> On 26 October 2015 at 12:21, Doug Evans <xdje42@gmail.com> wrote:
>> The function comment for gdbscm_with_guile says:
>>
>> /* A wrapper around scm_with_guile that prints backtraces and exceptions
>>    according to "set guile print-stack".
>>    The result if NULL if no exception occurred, otherwise it is a statically
>>    allocated error message (caller must *not* free).  */
>>
>> If we're going to return an error message,
>> why make it a void * and not a char * (const as appropriate)?
>>
>> The lower level guile API uses a void * because it doesn't specify what
>> the result is. But in this use of it we do specify what the result is.
>
> It totally makes sense.  Here is the updated patch (probably garbled
> by gmail, sorry about that):
>
>
> From 2defdf7de52146ea7508d0dda2f0c59e7fdd42fe Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon dot marchi at polymtl dot ca>
> Date: Sun, 25 Oct 2015 23:46:37 -0400
> Subject: [PATCH] guile: Change return value of gdbscm_with_guile for const
>  char *
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The documentation of gdbscm_with_guile says that it returns a statically
> allocated string (IOW, a const char *).  We can reflect that in its
> return value type, and get rid of C++ build errors.
>
> Initially fixes:
>
> /home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c: In function
> ‘void* gdbscm_disasm_read_memory_worker(void*)’:
> /home/simark/src/binutils-gdb/gdb/guile/scm-disasm.c:93:12: error:
> invalid conversion from ‘const void*’ to ‘void*’ [-fpermissive]
>      return "seek error";
>
> gdb/ChangeLog:
>
>     * guile/guile-internal.h (gdbscm_with_guile): Change return
>     types to const char *.
>     * guile/scm-safe-call.c (gdbscm_with_guile): Likewise.
>     (struct c_data) <func>: Likewise.
>     (struct c_data) <result>: Change type to const char *.
>     (scscm_eval_scheme_string): Change return type to
>     const char *.
>     (scscm_source_scheme_script): Likewise.
>     (gdbscm_safe_eval_string): Change type of result variable to
>     const char * and remove cast.
>     (gdbscm_safe_source_script): Likewise.
>     * guile/scm-disasm.c (gdbscm_disasm_read_memory_worker):
>     Change return type to const char *.
>     (gdbscm_disasm_read_memory): Change type of status to
>     const char *.

LGTM.
Thanks!

Re: [PATCH c++ 05/12] guile: Constify gdbscm_with_guile return value

[Simon Marchi]

On 27 October 2015 at 11:54, Doug Evans <xdje42@gmail.com> wrote:
> LGTM.
> Thanks!


Pushed, thanks!

[PATCH c++ 09/12] stap-probe.c: Add casts

[Simon Marchi]

gdb/ChangeLog:

	* stap-probe.c (handle_stap_probe): Add (const char *) casts.
---
 gdb/stap-probe.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index d88c470..18e0d83 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -1488,8 +1488,9 @@ handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
 
   /* Provider and the name of the probe.  */
   ret->p.provider = (char *) &el->data[3 * size];
-  ret->p.name = memchr (ret->p.provider, '\0',
-			(char *) el->data + el->size - ret->p.provider);
+  ret->p.name = ((const char *)
+		 memchr (ret->p.provider, '\0',
+			 (char *) el->data + el->size - ret->p.provider));
   /* Making sure there is a name.  */
   if (ret->p.name == NULL)
     {
@@ -1519,8 +1520,9 @@ handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
 
   /* Arguments.  We can only extract the argument format if there is a valid
      name for this probe.  */
-  probe_args = memchr (ret->p.name, '\0',
-		       (char *) el->data + el->size - ret->p.name);
+  probe_args = ((const char*)
+		memchr (ret->p.name, '\0',
+			(char *) el->data + el->size - ret->p.name));
 
   if (probe_args != NULL)
     ++probe_args;
-- 
2.6.2

Re: [PATCH c++ 09/12] stap-probe.c: Add casts

[Simon Marchi]

I pushed this one, I consider it obvious enough.

[PATCH c++ 10/12] symtab.c: Add cast

[Simon Marchi]

gdb/ChangeLog:

	* symtab.c (default_make_symbol_completion_list_break_on_1): Add
	cast.
---
 gdb/symtab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index b0a16b6..16190c4 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5422,7 +5422,7 @@ default_make_symbol_completion_list_break_on_1 (const char *text,
       /* These languages may have parameters entered by user but they are never
 	 present in the partial symbol tables.  */
 
-      const char *cs = memchr (sym_text, '(', sym_text_len);
+      const char *cs = (const char *) memchr (sym_text, '(', sym_text_len);
 
       if (cs)
 	sym_text_len = cs - sym_text;
-- 
2.6.2

Re: [PATCH c++ 10/12] symtab.c: Add cast

[Doug Evans]

Simon Marchi <simon.marchi@polymtl.ca> writes:
> gdb/ChangeLog:
>
> 	* symtab.c (default_make_symbol_completion_list_break_on_1): Add
> 	cast.
> ---
>  gdb/symtab.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index b0a16b6..16190c4 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -5422,7 +5422,7 @@ default_make_symbol_completion_list_break_on_1 (const char *text,
>        /* These languages may have parameters entered by user but they are never
>  	 present in the partial symbol tables.  */
>  
> -      const char *cs = memchr (sym_text, '(', sym_text_len);
> +      const char *cs = (const char *) memchr (sym_text, '(', sym_text_len);
>  
>        if (cs)
>  	sym_text_len = cs - sym_text;

LGTM

[PATCH c++ 04/12] Add scm_t_dynwind_flags casts

[Simon Marchi]

There is a handful of calls to

  scm_dynwind_begin (0);

where the parameter is an enum, scm_t_dynwind_flags.  In C++, we have no
choice but to add an explicit cast, since there is no enum value that
represents 0 (no flags set).

gdb/ChangeLog:

	* guile/scm-breakpoint.c (gdbscm_set_breakpoint_stop_x): Add
	scm_t_dynwind_flags casts.
	* guile/scm-cmd.c (gdbscm_parse_command_name): Likewise.
	* guile/scm-ports.c (gdbscm_open_memory): Likewise.
	* guile/scm-value.c (gdbscm_value_to_string): Likewise.
---
 gdb/guile/scm-breakpoint.c | 2 +-
 gdb/guile/scm-cmd.c        | 4 ++--
 gdb/guile/scm-ports.c      | 2 +-
 gdb/guile/scm-value.c      | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index 83574a3..b599e23 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -956,7 +956,7 @@ gdbscm_set_breakpoint_stop_x (SCM self, SCM newvalue)
 			" this breakpoint."),
 		      ext_lang_capitalized_name (extlang));
 
-      scm_dynwind_begin (0);
+      scm_dynwind_begin ((scm_t_dynwind_flags) 0);
       gdbscm_dynwind_xfree (error_text);
       gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, self, error_text);
       /* The following line, while unnecessary, is present for completeness
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 900d7b0..051f0d2 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -530,7 +530,7 @@ gdbscm_parse_command_name (const char *name,
       msg = xstrprintf (_("could not find command prefix '%s'"), prefix_text);
       xfree (prefix_text);
       xfree (result);
-      scm_dynwind_begin (0);
+      scm_dynwind_begin ((scm_t_dynwind_flags) 0);
       gdbscm_dynwind_xfree (msg);
       gdbscm_out_of_range_error (func_name, arg_pos,
 				 gdbscm_scm_from_c_string (name), msg);
@@ -546,7 +546,7 @@ gdbscm_parse_command_name (const char *name,
   msg = xstrprintf (_("'%s' is not a prefix command"), prefix_text);
   xfree (prefix_text);
   xfree (result);
-  scm_dynwind_begin (0);
+  scm_dynwind_begin ((scm_t_dynwind_flags) 0);
   gdbscm_dynwind_xfree (msg);
   gdbscm_out_of_range_error (func_name, arg_pos,
 			     gdbscm_scm_from_c_string (name), msg);
diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 90bdb39..925f3b2 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -1136,7 +1136,7 @@ gdbscm_open_memory (SCM rest)
 			      &start_arg_pos, &start,
 			      &size_arg_pos, &size);
 
-  scm_dynwind_begin (0);
+  scm_dynwind_begin ((scm_t_dynwind_flags) 0);
 
   if (mode == NULL)
     mode = xstrdup ("r");
diff --git a/gdb/guile/scm-value.c b/gdb/guile/scm-value.c
index f25f7d5..851d8a7 100644
--- a/gdb/guile/scm-value.c
+++ b/gdb/guile/scm-value.c
@@ -1158,7 +1158,7 @@ gdbscm_value_to_string (SCM self, SCM rest)
      Make sure we don't leak.  This is done via scm_dynwind_begin, et.al.  */
   discard_cleanups (cleanups);
 
-  scm_dynwind_begin (0);
+  scm_dynwind_begin ((scm_t_dynwind_flags) 0);
 
   gdbscm_dynwind_xfree (encoding);
   gdbscm_dynwind_xfree (buffer);
-- 
2.6.2

Re: [PATCH c++ 04/12] Add scm_t_dynwind_flags casts

[Pedro Alves]

On 10/26/2015 03:46 AM, Simon Marchi wrote:
> There is a handful of calls to
> 
>   scm_dynwind_begin (0);
> 
> where the parameter is an enum, scm_t_dynwind_flags.  In C++, we have no
> choice but to add an explicit cast, since there is no enum value that
> represents 0 (no flags set).

I agree; I don't think there's anything else that we could do here.

So, OK.

Thanks,
Pedro Alves

Re: [PATCH c++ 04/12] Add scm_t_dynwind_flags casts

[Simon Marchi]

On 27/10/15 08:15 AM, Pedro Alves wrote:
> I agree; I don't think there's anything else that we could do here.
> 
> So, OK.
> 
> Thanks,
> Pedro Alves

Pushed, thanks.

[PATCH c++ 07/12] gdbscm_memory_port_write: use local variable to avoid adding casts

[Simon Marchi]

By having a local variable of type (const gdb_byte *), we can avoid adding
two casts.

gdb/ChangeLog:

	* guile/scm-ports.c (gdbscm_memory_port_write): Declare new
	"data" local variable and use it.
---
 gdb/guile/scm-ports.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 49e4fd6..2cca889 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -725,10 +725,11 @@ gdbscm_memory_port_flush (SCM port)
 /* "write" method for memory ports.  */
 
 static void
-gdbscm_memory_port_write (SCM port, const void *data, size_t size)
+gdbscm_memory_port_write (SCM port, const void *void_data, size_t size)
 {
   scm_t_port *pt = SCM_PTAB_ENTRY (port);
   ioscm_memory_port *iomem = (ioscm_memory_port *) SCM_STREAM (port);
+  const gdb_byte *data = (const gdb_byte *) void_data;
 
   /* There's no way to indicate a short write, so if the request goes past
      the end of the port's memory range, flag an error.  */
@@ -767,7 +768,7 @@ gdbscm_memory_port_write (SCM port, const void *data, size_t size)
 	pt->write_pos = pt->write_end;
 	gdbscm_memory_port_flush (port);
 	{
-	  const void *ptr = ((const char *) data) + space;
+	  const gdb_byte *ptr = data + space;
 	  size_t remaining = size - space;
 
 	  if (remaining >= pt->write_buf_size)
-- 
2.6.2

Re: [PATCH c++ 07/12] gdbscm_memory_port_write: use local variable to avoid adding casts

[Doug Evans]

Simon Marchi <simon.marchi@polymtl.ca> writes:
> By having a local variable of type (const gdb_byte *), we can avoid adding
> two casts.
>
> gdb/ChangeLog:
>
> 	* guile/scm-ports.c (gdbscm_memory_port_write): Declare new
> 	"data" local variable and use it.
> ---
>  gdb/guile/scm-ports.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
> index 49e4fd6..2cca889 100644
> --- a/gdb/guile/scm-ports.c
> +++ b/gdb/guile/scm-ports.c
> @@ -725,10 +725,11 @@ gdbscm_memory_port_flush (SCM port)
>  /* "write" method for memory ports.  */
>  
>  static void
> -gdbscm_memory_port_write (SCM port, const void *data, size_t size)
> +gdbscm_memory_port_write (SCM port, const void *void_data, size_t size)
>  {
>    scm_t_port *pt = SCM_PTAB_ENTRY (port);
>    ioscm_memory_port *iomem = (ioscm_memory_port *) SCM_STREAM (port);
> +  const gdb_byte *data = (const gdb_byte *) void_data;
>  
>    /* There's no way to indicate a short write, so if the request goes past
>       the end of the port's memory range, flag an error.  */
> @@ -767,7 +768,7 @@ gdbscm_memory_port_write (SCM port, const void *data, size_t size)
>  	pt->write_pos = pt->write_end;
>  	gdbscm_memory_port_flush (port);
>  	{
> -	  const void *ptr = ((const char *) data) + space;
> +	  const gdb_byte *ptr = data + space;
>  	  size_t remaining = size - space;
>  
>  	  if (remaining >= pt->write_buf_size)

LGTM

[PATCH c++ 02/12] ctf.c: Fix int/enum implicit cast

[Simon Marchi]

This patch was taken directly from Pedro's branch.

Right now, SET_INT32_FIELD is used to set enum fields.  This works in C,
but not C++.  Therefore, define the new SET_ENUM_FIELD, which casts the
value to the right enum type.

gdb/ChangeLog:

	* ctf.c (SET_ENUM_FIELD): New macro.
	(ctf_read_status): Use it.
	(ctf_read_tp): Use it.
---
 gdb/ctf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/ctf.c b/gdb/ctf.c
index 6c1aede..cb0d707 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -914,6 +914,12 @@ ctf_open_dir (const char *dirname)
 							   (SCOPE),	\
 							   #FIELD))
 
+#define SET_ENUM_FIELD(EVENT, SCOPE, VAR, TYPE, FIELD)			\
+  (VAR)->FIELD = (TYPE) bt_ctf_get_int64 (bt_ctf_get_field ((EVENT),	\
+							    (SCOPE),	\
+							    #FIELD))
+
+
 /* EVENT is the "status" event and TS is filled in.  */
 
 static void
@@ -922,7 +928,7 @@ ctf_read_status (struct bt_ctf_event *event, struct trace_status *ts)
   const struct bt_definition *scope
     = bt_ctf_get_top_level_scope (event, BT_EVENT_FIELDS);
 
-  SET_INT32_FIELD (event, scope, ts, stop_reason);
+  SET_ENUM_FIELD (event, scope, ts, enum trace_stop_reason, stop_reason);
   SET_INT32_FIELD (event, scope, ts, stopping_tracepoint);
   SET_INT32_FIELD (event, scope, ts, traceframe_count);
   SET_INT32_FIELD (event, scope, ts, traceframes_created);
@@ -1058,7 +1064,7 @@ ctf_read_tp (struct uploaded_tp **uploaded_tps)
       SET_INT32_FIELD (event, scope, utp, step);
       SET_INT32_FIELD (event, scope, utp, pass);
       SET_INT32_FIELD (event, scope, utp, hit_count);
-      SET_INT32_FIELD (event, scope, utp, type);
+      SET_ENUM_FIELD (event, scope, utp, enum bptype, type);
 
       /* Read 'cmd_strings'.  */
       SET_ARRAY_FIELD (event, scope, utp, cmd_num, cmd_strings);
-- 
2.6.2

Re: [PATCH c++ 02/12] ctf.c: Fix int/enum implicit cast

[Pedro Alves]

Yao, are you OK with this?  ( I obviously am :-) )

Thanks,
Pedro Alves

On 10/26/2015 03:46 AM, Simon Marchi wrote:
> This patch was taken directly from Pedro's branch.
> 
> Right now, SET_INT32_FIELD is used to set enum fields.  This works in C,
> but not C++.  Therefore, define the new SET_ENUM_FIELD, which casts the
> value to the right enum type.
> 
> gdb/ChangeLog:
> 
> 	* ctf.c (SET_ENUM_FIELD): New macro.
> 	(ctf_read_status): Use it.
> 	(ctf_read_tp): Use it.
> ---
>  gdb/ctf.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/ctf.c b/gdb/ctf.c
> index 6c1aede..cb0d707 100644
> --- a/gdb/ctf.c
> +++ b/gdb/ctf.c
> @@ -914,6 +914,12 @@ ctf_open_dir (const char *dirname)
>  							   (SCOPE),	\
>  							   #FIELD))
>  
> +#define SET_ENUM_FIELD(EVENT, SCOPE, VAR, TYPE, FIELD)			\
> +  (VAR)->FIELD = (TYPE) bt_ctf_get_int64 (bt_ctf_get_field ((EVENT),	\
> +							    (SCOPE),	\
> +							    #FIELD))
> +
> +
>  /* EVENT is the "status" event and TS is filled in.  */
>  
>  static void
> @@ -922,7 +928,7 @@ ctf_read_status (struct bt_ctf_event *event, struct trace_status *ts)
>    const struct bt_definition *scope
>      = bt_ctf_get_top_level_scope (event, BT_EVENT_FIELDS);
>  
> -  SET_INT32_FIELD (event, scope, ts, stop_reason);
> +  SET_ENUM_FIELD (event, scope, ts, enum trace_stop_reason, stop_reason);
>    SET_INT32_FIELD (event, scope, ts, stopping_tracepoint);
>    SET_INT32_FIELD (event, scope, ts, traceframe_count);
>    SET_INT32_FIELD (event, scope, ts, traceframes_created);
> @@ -1058,7 +1064,7 @@ ctf_read_tp (struct uploaded_tp **uploaded_tps)
>        SET_INT32_FIELD (event, scope, utp, step);
>        SET_INT32_FIELD (event, scope, utp, pass);
>        SET_INT32_FIELD (event, scope, utp, hit_count);
> -      SET_INT32_FIELD (event, scope, utp, type);
> +      SET_ENUM_FIELD (event, scope, utp, enum bptype, type);
>  
>        /* Read 'cmd_strings'.  */
>        SET_ARRAY_FIELD (event, scope, utp, cmd_num, cmd_strings);
> 

Re: [PATCH c++ 02/12] ctf.c: Fix int/enum implicit cast

[Yao Qi]

On 27/10/15 12:18, Pedro Alves wrote:
> Yao, are you OK with this?  ( I obviously am:-)  )

OK to me.

-- 
Yao (齐尧)

Re: [PATCH c++ 02/12] ctf.c: Fix int/enum implicit cast

[Simon Marchi]

On 27 October 2015 at 10:01, Yao Qi <qiyaoltc@gmail.com> wrote:
> OK to me.


Thanks, pushed.

Re: [PATCH c++ 01/12] Introduce ax_raw_byte and use it

[Pedro Alves]

On 10/26/2015 03:46 AM, Simon Marchi wrote:
> This patch was taken directly from Pedro's branch.
> 

Thanks.

> ax_simple is used to append an agent expression operator to an agent
> expression string.  Therefore, it takes an enum agent_op as input.
> There is an instance where it's called to append a raw byte, unrelated
> to the enum.  It makes the build fail in C++ mode.
> 
> This patch introduces ax_raw_byte for that purpose and uses it.
> 
> gdb/ChangeLog:
> 
> 	* ax.h (ax_raw_byte): New declaration.
> 	* ax-general.c (ax_raw_byte): New function.
> 	(ax_simple): Use ax_raw_byte.
> 	* ax-gdb.c (gen_printf): Likewise.

OK.

Thanks,
Pedro Alves

Re: [PATCH c++ 01/12] Introduce ax_raw_byte and use it

[Simon Marchi]

On 27/10/15 08:16 AM, Pedro Alves wrote:
> On 10/26/2015 03:46 AM, Simon Marchi wrote:
>> This patch was taken directly from Pedro's branch.
>>
> 
> Thanks.
> 
>> ax_simple is used to append an agent expression operator to an agent
>> expression string.  Therefore, it takes an enum agent_op as input.
>> There is an instance where it's called to append a raw byte, unrelated
>> to the enum.  It makes the build fail in C++ mode.
>>
>> This patch introduces ax_raw_byte for that purpose and uses it.
>>
>> gdb/ChangeLog:
>>
>> 	* ax.h (ax_raw_byte): New declaration.
>> 	* ax-general.c (ax_raw_byte): New function.
>> 	(ax_simple): Use ax_raw_byte.
>> 	* ax-gdb.c (gen_printf): Likewise.
> 
> OK.
> 
> Thanks,
> Pedro Alves
> 

Pushed thanks.