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

Message ID 20200128142830.89282-1-ldurfina@tachyum.com
State New
Headers show
Series
  • [1/2] gdbsupport: Fix a type of sentinel
Related show

Commit Message

Lukas Durfina Jan. 28, 2020, 2:28 p.m.
---
 gdbsupport/ChangeLog | 4 ++++
 gdbsupport/environ.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.17.1

Comments

Simon Marchi Jan. 28, 2020, 4:43 p.m. | #1
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
Tom Tromey Jan. 31, 2020, 10:09 a.m. | #2
>>>>> "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
Simon Marchi Jan. 31, 2020, 4:52 p.m. | #3
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

Patch

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