[PATCH 1/2] gdbsupport: Fix a type of sentinel

[PATCH 1/2] gdbsupport: Fix a type of sentinel

[Lukas Durfina]

---
 gdbsupport/ChangeLog | 4 ++++
 gdbsupport/environ.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
index 6ea2f2c253..3c77483294 100644
--- a/gdbsupport/ChangeLog
+++ b/gdbsupport/ChangeLog
@@ -1,3 +1,7 @@
+2020-01-28 Lukas Durfina <ldurfina@tachyum.com>
+
+	* environ.c (gdb_environ::set): Fix a type of sentinel.
+
 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
 
 	* thread-pool.c (set_thread_name): Add an overload for the NetBSD
diff --git a/gdbsupport/environ.c b/gdbsupport/environ.c
index 55d0a74c37..32434ee0b7 100644
--- a/gdbsupport/environ.c
+++ b/gdbsupport/environ.c
@@ -105,7 +105,7 @@ gdb_environ::get (const char *var) const
 void
 gdb_environ::set (const char *var, const char *value)
 {
-  char *fullvar = concat (var, "=", value, NULL);
+  char *fullvar = concat (var, "=", value, (char *) NULL);
 
   /* We have to unset the variable in the vector if it exists.  */
   unset (var, false);
-- 
2.17.1

[PATCH 2/2] gdb: Fix a type of sentinel

[Lukas Durfina]

---
 gdb/ChangeLog | 4 ++++
 gdb/charset.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8b6da62bd4..1dd89ae2e0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-01-28 Lukas Durfina <ldurfina@tachyum.com>
+
+	* charset.c (find_charset_names): Fix a type of sentinel.
+
 2020-01-26  Tom Tromey  <tom@tromey.com>
 
 	* ctfread.c (struct ctf_fp_info): Reindent.
diff --git a/gdb/charset.c b/gdb/charset.c
index 5cfd2d8030..abf795ce6d 100644
--- a/gdb/charset.c
+++ b/gdb/charset.c
@@ -818,7 +818,7 @@ find_charset_names (void)
   {
     std::string iconv_dir = relocate_gdb_directory (ICONV_BIN,
 						    ICONV_BIN_RELOCATABLE);
-    iconv_program = concat (iconv_dir.c_str(), SLASH_STRING, "iconv", NULL);
+    iconv_program = concat (iconv_dir.c_str(), SLASH_STRING, "iconv", (char *) NULL);
   }
 #else
   iconv_program = xstrdup ("iconv");
-- 
2.17.1

Re: [PATCH 2/2] gdb: Fix a type of sentinel

[Simon Marchi]

On 2020-01-28 9:28 a.m., Lukas Durfina wrote:
> ---
>  gdb/ChangeLog | 4 ++++
>  gdb/charset.c | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 8b6da62bd4..1dd89ae2e0 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-01-28 Lukas Durfina <ldurfina@tachyum.com>
> +
> +	* charset.c (find_charset_names): Fix a type of sentinel.
> +
>  2020-01-26  Tom Tromey  <tom@tromey.com>
>  
>  	* ctfread.c (struct ctf_fp_info): Reindent.
> diff --git a/gdb/charset.c b/gdb/charset.c
> index 5cfd2d8030..abf795ce6d 100644
> --- a/gdb/charset.c
> +++ b/gdb/charset.c
> @@ -818,7 +818,7 @@ find_charset_names (void)
>    {
>      std::string iconv_dir = relocate_gdb_directory (ICONV_BIN,
>  						    ICONV_BIN_RELOCATABLE);
> -    iconv_program = concat (iconv_dir.c_str(), SLASH_STRING, "iconv", NULL);
> +    iconv_program = concat (iconv_dir.c_str(), SLASH_STRING, "iconv", (char *) NULL);
>    }
>  #else
>    iconv_program = xstrdup ("iconv");
> -- 
> 2.17.1
> 

Hi Lukas,

Same comment as patch 1/2.

If this change fixes the same problem as patch 1/2, then you can put
both changes in the same patch.

Simon

RE: [PATCH 2/2] gdb: Fix a type of sentinel

[Aktemur, Tankut Baris]

On Tuesday, January 28, 2020 5:44 PM, Simon Marchi wrote:
> On 2020-01-28 9:28 a.m., Lukas Durfina wrote:
> > ---
> >  gdb/ChangeLog | 4 ++++
> >  gdb/charset.c | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > index 8b6da62bd4..1dd89ae2e0 100644
> > --- a/gdb/ChangeLog
> > +++ b/gdb/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2020-01-28 Lukas Durfina <ldurfina@tachyum.com>
> > +
> > +	* charset.c (find_charset_names): Fix a type of sentinel.
> > +
> >  2020-01-26  Tom Tromey  <tom@tromey.com>
> >
> >  	* ctfread.c (struct ctf_fp_info): Reindent.
> > diff --git a/gdb/charset.c b/gdb/charset.c
> > index 5cfd2d8030..abf795ce6d 100644
> > --- a/gdb/charset.c
> > +++ b/gdb/charset.c
> > @@ -818,7 +818,7 @@ find_charset_names (void)
> >    {
> >      std::string iconv_dir = relocate_gdb_directory (ICONV_BIN,
> >  						    ICONV_BIN_RELOCATABLE);
> > -    iconv_program = concat (iconv_dir.c_str(), SLASH_STRING, "iconv", NULL);
> > +    iconv_program = concat (iconv_dir.c_str(), SLASH_STRING, "iconv", (char *) NULL);
> >    }
> >  #else
> >    iconv_program = xstrdup ("iconv");
> > --
> > 2.17.1
> >
> 
> Hi Lukas,
> 
> Same comment as patch 1/2.
> 
> If this change fixes the same problem as patch 1/2, then you can put
> both changes in the same patch.
> 
> Simon

Now that the line is being touched, it might also be a good idea to replace
NULL with nullptr.

-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 2/2] gdb: Fix a type of sentinel

[Lukas Durfina]

Hello,

nullptr works of course too. I will prepare the second version with nullptr.

To Simon: I will merge both patches into the single one.
Also, I will update the commit message about fixed compilation error.
You are right, it fixes the issue for cross compiling, but for our new architecture (Tachyum) that is being developed.

I assume the change is generic improvement.

Thanks
Lukas


> -----Original Message-----
> From: Aktemur, Tankut Baris <tankut.baris.aktemur@intel.com>
> Sent: Tuesday, January 28, 2020 6:06 PM
> To: Simon Marchi <simark@simark.ca>; Lukas Durfina
> <ldurfina@tachyum.com>; gdb-patches@sourceware.org
> Subject: RE: [PATCH 2/2] gdb: Fix a type of sentinel
> 
> On Tuesday, January 28, 2020 5:44 PM, Simon Marchi wrote:
> > On 2020-01-28 9:28 a.m., Lukas Durfina wrote:
> > > ---
> > >  gdb/ChangeLog | 4 ++++
> > >  gdb/charset.c | 2 +-
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog index
> > > 8b6da62bd4..1dd89ae2e0 100644
> > > --- a/gdb/ChangeLog
> > > +++ b/gdb/ChangeLog
> > > @@ -1,3 +1,7 @@
> > > +2020-01-28 Lukas Durfina <ldurfina@tachyum.com>
> > > +
> > > +	* charset.c (find_charset_names): Fix a type of sentinel.
> > > +
> > >  2020-01-26  Tom Tromey  <tom@tromey.com>
> > >
> > >  	* ctfread.c (struct ctf_fp_info): Reindent.
> > > diff --git a/gdb/charset.c b/gdb/charset.c index
> > > 5cfd2d8030..abf795ce6d 100644
> > > --- a/gdb/charset.c
> > > +++ b/gdb/charset.c
> > > @@ -818,7 +818,7 @@ find_charset_names (void)
> > >    {
> > >      std::string iconv_dir = relocate_gdb_directory (ICONV_BIN,
> > >
> ICONV_BIN_RELOCATABLE);
> > > -    iconv_program = concat (iconv_dir.c_str(), SLASH_STRING, "iconv",
> NULL);
> > > +    iconv_program = concat (iconv_dir.c_str(), SLASH_STRING,
> > > + "iconv", (char *) NULL);
> > >    }
> > >  #else
> > >    iconv_program = xstrdup ("iconv");
> > > --
> > > 2.17.1
> > >
> >
> > Hi Lukas,
> >
> > Same comment as patch 1/2.
> >
> > If this change fixes the same problem as patch 1/2, then you can put
> > both changes in the same patch.
> >
> > Simon
> 
> Now that the line is being touched, it might also be a good idea to replace
> NULL with nullptr.
> 
> -Baris
> 
> 
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de
> Managing Directors: Christin Eisenschmid, Gary Kershaw Chairperson of the
> Supervisory Board: Nicole Lau Registered Office: Munich Commercial
> Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 1/2] gdbsupport: Fix a type of sentinel

[Simon Marchi]

On 2020-01-28 9:28 a.m., Lukas Durfina wrote:
> ---
>  gdbsupport/ChangeLog | 4 ++++
>  gdbsupport/environ.c | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
> index 6ea2f2c253..3c77483294 100644
> --- a/gdbsupport/ChangeLog
> +++ b/gdbsupport/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-01-28 Lukas Durfina <ldurfina@tachyum.com>
> +
> +	* environ.c (gdb_environ::set): Fix a type of sentinel.
> +
>  2020-01-24  Christian Biesinger  <cbiesinger@google.com>
>  
>  	* thread-pool.c (set_thread_name): Add an overload for the NetBSD
> diff --git a/gdbsupport/environ.c b/gdbsupport/environ.c
> index 55d0a74c37..32434ee0b7 100644
> --- a/gdbsupport/environ.c
> +++ b/gdbsupport/environ.c
> @@ -105,7 +105,7 @@ gdb_environ::get (const char *var) const
>  void
>  gdb_environ::set (const char *var, const char *value)
>  {
> -  char *fullvar = concat (var, "=", value, NULL);
> +  char *fullvar = concat (var, "=", value, (char *) NULL);
>  
>    /* We have to unset the variable in the vector if it exists.  */
>    unset (var, false);
> -- 
> 2.17.1
> 

Hi Lukas,

Can you please indicate in your commit message the reason why you need this
change?  I presume it fixes some compilation error, in which case please
indicate at least what is the error message and what is the compiler you are
using (name and version).  If you are cross compiling, it can also be good
to mention that as well.

Simon

Re: [PATCH 1/2] gdbsupport: Fix a type of sentinel

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Can you please indicate in your commit message the reason why you need this
Simon> change?  I presume it fixes some compilation error, in which case please
Simon> indicate at least what is the error message and what is the compiler you are
Simon> using (name and version).  If you are cross compiling, it can also be good
Simon> to mention that as well.

The bug here is that if NULL is defined as 0, then the sentinel will be
handled incorrectly via varargs if sizeof(void*) > sizeof(int) -- the
value passed in will be narrower than what the callee reads.  Jan did a
pass once, ages ago, to fix all of these, but it's easy for new ones to
slip in, since it's not always incorrect.

I like the nullptr idea.  Also replacing concat calls with std::string
in appropriate spots seems like a good idea too.

Tom

Re: [PATCH 1/2] gdbsupport: Fix a type of sentinel

[Simon Marchi]

On 2020-01-31 5:09 a.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> Can you please indicate in your commit message the reason why you need this
> Simon> change?  I presume it fixes some compilation error, in which case please
> Simon> indicate at least what is the error message and what is the compiler you are
> Simon> using (name and version).  If you are cross compiling, it can also be good
> Simon> to mention that as well.
> 
> The bug here is that if NULL is defined as 0, then the sentinel will be
> handled incorrectly via varargs if sizeof(void*) > sizeof(int) -- the
> value passed in will be narrower than what the callee reads.  Jan did a
> pass once, ages ago, to fix all of these, but it's easy for new ones to
> slip in, since it's not always incorrect.

Ok thanks, I didn't know about that.  I think that this information should
end up in the commit message.

> I like the nullptr idea.  Also replacing concat calls with std::string
> in appropriate spots seems like a good idea too.

Indeed.

Simon