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

Message ID CAFXXi0mDYRDkMegxy9KgvtezjFdz+Btr6kHFa44m4Sp6vybquw@mail.gmail.com
State New, archived
Headers

Commit Message

Simon Marchi Oct. 26, 2015, 5:27 p.m. UTC
  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(-)
  

Comments

Pedro Alves Oct. 26, 2015, 5:39 p.m. UTC | #1
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
  
Doug Evans Oct. 26, 2015, 6:36 p.m. UTC | #2
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.
  
Simon Marchi Oct. 26, 2015, 7:06 p.m. UTC | #3
On 26 October 2015 at 14:36, Doug Evans <xdje42@gmail.com> wrote:
> Fine by me.

Thanks, pushed.
  

Patch

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