public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [review] Use strtok_r instead of strtok
@ 2019-11-02 17:27 Christian Biesinger (Code Review)
  2019-11-06  0:46 ` Pedro Alves (Code Review)
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-02 17:27 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/484
......................................................................

Use strtok_r instead of strtok

Improves threadsafety. Guile and Python can create threads, and the patch
series at https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/176
will make GDB create threads too.

gdb/ChangeLog:

2019-11-02  Christian Biesinger  <cbiesinger@google.com>

	* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
	* mi/mi-main.c (output_cores): Likewise.
	* nat/linux-osdata.c (linux_xfer_osdata_cpus): Likewise.
	(linux_xfer_osdata_modules): Likewise.
	* remote.c (register_remote_support_xml): Likewise.
	* sparc64-tdep.c (adi_is_addr_mapped): Likewise.
	* xml-syscall.c (syscall_create_syscall_desc): Likewise.

gdb/gdbserver/ChangeLog:

2019-11-02  Christian Biesinger  <cbiesinger@google.com>

	* linux-x86-low.c (x86_linux_process_qsupported): Use strtok_r
	instead of strtok.
	* server.c (handle_query): Likewise.
	(captured_main): Likewise.

Change-Id: Ief6138965a24398e5fc064598cd8f2abd3b5047c
---
M gdb/gdbserver/linux-x86-low.c
M gdb/gdbserver/server.c
M gdb/linux-tdep.c
M gdb/mi/mi-main.c
M gdb/nat/linux-osdata.c
M gdb/remote.c
M gdb/sparc64-tdep.c
M gdb/xml-syscall.c
8 files changed, 36 insertions(+), 27 deletions(-)



diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index cafff6b..54bd2a2 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -912,9 +912,11 @@
       if (startswith (feature, "xmlRegisters="))
 	{
 	  char *copy = xstrdup (feature + 13);
-	  char *p;
 
-	  for (p = strtok (copy, ","); p != NULL; p = strtok (NULL, ","))
+	  char *saveptr;
+	  for (char *p = strtok_r (copy, ",", &saveptr);
+	       p != NULL;
+	       p = strtok_r (NULL, ",", &saveptr))
 	    {
 	      if (strcmp (p, "i386") == 0)
 		{
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 59e8a55..c5f7176 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2268,9 +2268,10 @@
 
 	  /* Two passes, to avoid nested strtok calls in
 	     target_process_qsupported.  */
-	  for (p = strtok (p + 1, ";");
+	  char *saveptr;
+	  for (p = strtok_r (p + 1, ";", &saveptr);
 	       p != NULL;
-	       p = strtok (NULL, ";"))
+	       p = strtok_r (NULL, ";", &saveptr))
 	    {
 	      count++;
 	      qsupported = XRESIZEVEC (char *, qsupported, count);
@@ -3633,12 +3634,11 @@
 	}
       else if (startswith (*next_arg, "--disable-packet="))
 	{
-	  char *packets, *tok;
-
-	  packets = *next_arg += sizeof ("--disable-packet=") - 1;
-	  for (tok = strtok (packets, ",");
+	  char *packets = *next_arg += sizeof ("--disable-packet=") - 1;
+	  char *saveptr;
+	  for (char *tok = strtok_r (packets, ",", &saveptr);
 	       tok != NULL;
-	       tok = strtok (NULL, ","))
+	       tok = strtok_r (NULL, ",", &saveptr))
 	    {
 	      if (strcmp ("vCont", tok) == 0)
 		disable_packet_vCont = true;
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 567b01c..18cee91 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -839,9 +839,10 @@
 			   "      Size", "    Offset", "objfile");
 	    }
 
-	  for (line = strtok (map.get (), "\n");
+	  char *saveptr;
+	  for (line = strtok_r (map.get (), "\n", &saveptr);
 	       line;
-	       line = strtok (NULL, "\n"))
+	       line = strtok_r (NULL, "\n", &saveptr))
 	    {
 	      ULONGEST addr, endaddr, offset, inode;
 	      const char *permissions, *device, *mapping_filename;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 0e99fa3..c14897a 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -696,8 +696,9 @@
   ui_out_emit_list list_emitter (uiout, field_name);
   auto cores = make_unique_xstrdup (xcores);
   char *p = cores.get ();
+  char *saveptr;
 
-  for (p = strtok (p, ","); p;  p = strtok (NULL, ","))
+  for (p = strtok_r (p, ",", &saveptr); p;  p = strtok_r (NULL, ",", &saveptr))
     uiout->field_string (NULL, p);
 }
 
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 67f9f3a..84357e2 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -566,11 +566,12 @@
 	      char *key, *value;
 	      int i = 0;
 
-	      key = strtok (buf, ":");
+	      char *saveptr;
+	      key = strtok_r (buf, ":", &saveptr);
 	      if (key == NULL)
 		continue;
 
-	      value = strtok (NULL, ":");
+	      value = strtok_r (NULL, ":", &saveptr);
 	      if (value == NULL)
 		continue;
 
@@ -1216,36 +1217,36 @@
 	{
 	  if (fgets (buf, sizeof (buf), fp.get ()))
 	    {
-	      char *name, *dependencies, *status, *tmp;
+	      char *name, *dependencies, *status, *tmp, *saveptr;
 	      unsigned int size;
 	      unsigned long long address;
 	      int uses;
 
-	      name = strtok (buf, " ");
+	      name = strtok_r (buf, " ", &saveptr);
 	      if (name == NULL)
 		continue;
 
-	      tmp = strtok (NULL, " ");
+	      tmp = strtok_r (NULL, " ", &saveptr);
 	      if (tmp == NULL)
 		continue;
 	      if (sscanf (tmp, "%u", &size) != 1)
 		continue;
 
-	      tmp = strtok (NULL, " ");
+	      tmp = strtok_r (NULL, " ", &saveptr);
 	      if (tmp == NULL)
 		continue;
 	      if (sscanf (tmp, "%d", &uses) != 1)
 		continue;
 
-	      dependencies = strtok (NULL, " ");
+	      dependencies = strtok_r (NULL, " ", &saveptr);
 	      if (dependencies == NULL)
 		continue;
 
-	      status = strtok (NULL, " ");
+	      status = strtok_r (NULL, " ", &saveptr);
 	      if (status == NULL)
 		continue;
 
-	      tmp = strtok (NULL, "\n");
+	      tmp = strtok_r (NULL, "\n", &saveptr);
 	      if (tmp == NULL)
 		continue;
 	      if (sscanf (tmp, "%llx", &address) != 1)
diff --git a/gdb/remote.c b/gdb/remote.c
index 8ea52d3..1ac9013 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5169,7 +5169,8 @@
   else
     {
       char *copy = xstrdup (remote_support_xml + 13);
-      char *p = strtok (copy, ",");
+      char *saveptr;
+      char *p = strtok_r (copy, ",", &saveptr);
 
       do
 	{
@@ -5180,7 +5181,7 @@
 	      return;
 	    }
 	}
-      while ((p = strtok (NULL, ",")) != NULL);
+      while ((p = strtok_r (NULL, ",", &saveptr)) != NULL);
       xfree (copy);
 
       remote_support_xml = reconcat (remote_support_xml,
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 873fbaa..fc73b23 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -316,8 +316,10 @@
   if (data)
     {
       adi_stat_t adi_stat = get_adi_info (pid);
-      char *line;
-      for (line = strtok (data.get (), "\n"); line; line = strtok (NULL, "\n"))
+      char *saveptr;
+      for (char *line = strtok_r (data.get (), "\n", &saveptr);
+	   line;
+	   line = strtok_r (NULL, "\n", &saveptr))
         {
           ULONGEST addr, endaddr;
 
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index dc988df..3830faa 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -221,9 +221,10 @@
   /*  Add syscall to its groups.  */
   if (groups != NULL)
     {
-      for (char *group = strtok (groups, ",");
+      char *saveptr;
+      for (char *group = strtok_r (groups, ",", &saveptr);
 	   group != NULL;
-	   group = strtok (NULL, ","))
+	   group = strtok_r (NULL, ",", &saveptr))
 	syscall_group_add_syscall (syscalls_info, sysdesc, group);
     }
 }

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ief6138965a24398e5fc064598cd8f2abd3b5047c
Gerrit-Change-Number: 484
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: newchange

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

* [review] Use strtok_r instead of strtok
  2019-11-02 17:27 [review] Use strtok_r instead of strtok Christian Biesinger (Code Review)
@ 2019-11-06  0:46 ` Pedro Alves (Code Review)
  2019-11-06 20:03 ` Christian Biesinger (Code Review)
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Pedro Alves (Code Review) @ 2019-11-06  0:46 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches

Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/484
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

I double-checked that we're pulling the strtok_r module in gnulib, so portability isn't an issue.

This LGTM.  Thanks for doing this.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/484/1//COMMIT_MSG 
Commit Message:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/484/1//COMMIT_MSG@11 
PS1, Line 11: 
 6 | 
 7 | Use strtok_r instead of strtok
 8 | 
 9 | Improves threadsafety. Guile and Python can create threads, and the patch
10 | series at https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/176
11 > will make GDB create threads too.
12 | 
13 | gdb/ChangeLog:
14 | 
15 | 2019-11-02  Christian Biesinger  <cbiesinger@google.com>
16 | 

One would hope that using strtok in our code has no effect to Guile and Python, unless they're using strtok themselves, which I really hope they're not, since that would be obviously broken. E.g., if multiple python threads end up in strtok.  I'd bet they don't do that.

This is mainly useful to protect against multiple _gdb_ threads using strtok.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ief6138965a24398e5fc064598cd8f2abd3b5047c
Gerrit-Change-Number: 484
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 00:46:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

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

* [review] Use strtok_r instead of strtok
  2019-11-02 17:27 [review] Use strtok_r instead of strtok Christian Biesinger (Code Review)
  2019-11-06  0:46 ` Pedro Alves (Code Review)
@ 2019-11-06 20:03 ` Christian Biesinger (Code Review)
  2019-11-06 20:04 ` [pushed] " Sourceware to Gerrit sync (Code Review)
  2019-11-06 20:04 ` Sourceware to Gerrit sync (Code Review)
  3 siblings, 0 replies; 5+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-11-06 20:03 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches; +Cc: Pedro Alves

Christian Biesinger has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/484
......................................................................


Patch Set 1:

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/484/1//COMMIT_MSG 
Commit Message:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/484/1//COMMIT_MSG@11 
PS1, Line 11: 
 6 | 
 7 | Use strtok_r instead of strtok
 8 | 
 9 | Improves threadsafety. Guile and Python can create threads, and the patch
10 | series at https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/176
11 > will make GDB create threads too.
12 | 
13 | gdb/ChangeLog:
14 | 
15 | 2019-11-02  Christian Biesinger  <cbiesinger@google.com>
16 | 

> One would hope that using strtok in our code has no effect to Guile and Python, unless they're using […]

Yes, good point. Updated the commit message and pushed.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ief6138965a24398e5fc064598cd8f2abd3b5047c
Gerrit-Change-Number: 484
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 06 Nov 2019 20:03:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: comment

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

* [pushed] Use strtok_r instead of strtok
  2019-11-02 17:27 [review] Use strtok_r instead of strtok Christian Biesinger (Code Review)
  2019-11-06  0:46 ` Pedro Alves (Code Review)
  2019-11-06 20:03 ` Christian Biesinger (Code Review)
@ 2019-11-06 20:04 ` Sourceware to Gerrit sync (Code Review)
  2019-11-06 20:04 ` Sourceware to Gerrit sync (Code Review)
  3 siblings, 0 replies; 5+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-06 20:04 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches; +Cc: Pedro Alves

Sourceware to Gerrit sync has submitted this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/484
......................................................................

Use strtok_r instead of strtok

Improves threadsafety. This will be important when the patch series at
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/176
lands.

gdb/ChangeLog:

2019-11-06  Christian Biesinger  <cbiesinger@google.com>

	* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
	* mi/mi-main.c (output_cores): Likewise.
	* nat/linux-osdata.c (linux_xfer_osdata_cpus): Likewise.
	(linux_xfer_osdata_modules): Likewise.
	* remote.c (register_remote_support_xml): Likewise.
	* sparc64-tdep.c (adi_is_addr_mapped): Likewise.
	* xml-syscall.c (syscall_create_syscall_desc): Likewise.

gdb/gdbserver/ChangeLog:

2019-11-06  Christian Biesinger  <cbiesinger@google.com>

	* linux-x86-low.c (x86_linux_process_qsupported): Use strtok_r
	instead of strtok.
	* server.c (handle_query): Likewise.
	(captured_main): Likewise.

Change-Id: Ief6138965a24398e5fc064598cd8f2abd3b5047c
---
M gdb/ChangeLog
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/linux-x86-low.c
M gdb/gdbserver/server.c
M gdb/linux-tdep.c
M gdb/mi/mi-main.c
M gdb/nat/linux-osdata.c
M gdb/remote.c
M gdb/sparc64-tdep.c
M gdb/xml-syscall.c
10 files changed, 56 insertions(+), 27 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 048c2dd..f43d3a5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2019-11-06  Christian Biesinger  <cbiesinger@google.com>
+
+	* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
+	* mi/mi-main.c (output_cores): Likewise.
+	* nat/linux-osdata.c (linux_xfer_osdata_cpus): Likewise.
+	(linux_xfer_osdata_modules): Likewise.
+	* remote.c (register_remote_support_xml): Likewise.
+	* sparc64-tdep.c (adi_is_addr_mapped): Likewise.
+	* xml-syscall.c (syscall_create_syscall_desc): Likewise.
+
 2019-11-06  Tom Tromey  <tom@tromey.com>
 
 	* tui/tui-interp.c: Don't include readline.h.
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 35684db..9d0afaa 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,13 @@
+2019-11-06  Christian Biesinger  <cbiesinger@google.com>
+
+	* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
+	* mi/mi-main.c (output_cores): Likewise.
+	* nat/linux-osdata.c (linux_xfer_osdata_cpus): Likewise.
+	(linux_xfer_osdata_modules): Likewise.
+	* remote.c (register_remote_support_xml): Likewise.
+	* sparc64-tdep.c (adi_is_addr_mapped): Likewise.
+	* xml-syscall.c (syscall_create_syscall_desc): Likewise.
+
 2019-11-01  Christian Biesinger  <cbiesinger@google.com>
 
 	* configure: Regenerate.
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index cafff6b..54bd2a2 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -912,9 +912,11 @@
       if (startswith (feature, "xmlRegisters="))
 	{
 	  char *copy = xstrdup (feature + 13);
-	  char *p;
 
-	  for (p = strtok (copy, ","); p != NULL; p = strtok (NULL, ","))
+	  char *saveptr;
+	  for (char *p = strtok_r (copy, ",", &saveptr);
+	       p != NULL;
+	       p = strtok_r (NULL, ",", &saveptr))
 	    {
 	      if (strcmp (p, "i386") == 0)
 		{
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 59e8a55..c5f7176 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2268,9 +2268,10 @@
 
 	  /* Two passes, to avoid nested strtok calls in
 	     target_process_qsupported.  */
-	  for (p = strtok (p + 1, ";");
+	  char *saveptr;
+	  for (p = strtok_r (p + 1, ";", &saveptr);
 	       p != NULL;
-	       p = strtok (NULL, ";"))
+	       p = strtok_r (NULL, ";", &saveptr))
 	    {
 	      count++;
 	      qsupported = XRESIZEVEC (char *, qsupported, count);
@@ -3633,12 +3634,11 @@
 	}
       else if (startswith (*next_arg, "--disable-packet="))
 	{
-	  char *packets, *tok;
-
-	  packets = *next_arg += sizeof ("--disable-packet=") - 1;
-	  for (tok = strtok (packets, ",");
+	  char *packets = *next_arg += sizeof ("--disable-packet=") - 1;
+	  char *saveptr;
+	  for (char *tok = strtok_r (packets, ",", &saveptr);
 	       tok != NULL;
-	       tok = strtok (NULL, ","))
+	       tok = strtok_r (NULL, ",", &saveptr))
 	    {
 	      if (strcmp ("vCont", tok) == 0)
 		disable_packet_vCont = true;
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 567b01c..18cee91 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -839,9 +839,10 @@
 			   "      Size", "    Offset", "objfile");
 	    }
 
-	  for (line = strtok (map.get (), "\n");
+	  char *saveptr;
+	  for (line = strtok_r (map.get (), "\n", &saveptr);
 	       line;
-	       line = strtok (NULL, "\n"))
+	       line = strtok_r (NULL, "\n", &saveptr))
 	    {
 	      ULONGEST addr, endaddr, offset, inode;
 	      const char *permissions, *device, *mapping_filename;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 0e99fa3..c14897a 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -696,8 +696,9 @@
   ui_out_emit_list list_emitter (uiout, field_name);
   auto cores = make_unique_xstrdup (xcores);
   char *p = cores.get ();
+  char *saveptr;
 
-  for (p = strtok (p, ","); p;  p = strtok (NULL, ","))
+  for (p = strtok_r (p, ",", &saveptr); p;  p = strtok_r (NULL, ",", &saveptr))
     uiout->field_string (NULL, p);
 }
 
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 67f9f3a..84357e2 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -566,11 +566,12 @@
 	      char *key, *value;
 	      int i = 0;
 
-	      key = strtok (buf, ":");
+	      char *saveptr;
+	      key = strtok_r (buf, ":", &saveptr);
 	      if (key == NULL)
 		continue;
 
-	      value = strtok (NULL, ":");
+	      value = strtok_r (NULL, ":", &saveptr);
 	      if (value == NULL)
 		continue;
 
@@ -1216,36 +1217,36 @@
 	{
 	  if (fgets (buf, sizeof (buf), fp.get ()))
 	    {
-	      char *name, *dependencies, *status, *tmp;
+	      char *name, *dependencies, *status, *tmp, *saveptr;
 	      unsigned int size;
 	      unsigned long long address;
 	      int uses;
 
-	      name = strtok (buf, " ");
+	      name = strtok_r (buf, " ", &saveptr);
 	      if (name == NULL)
 		continue;
 
-	      tmp = strtok (NULL, " ");
+	      tmp = strtok_r (NULL, " ", &saveptr);
 	      if (tmp == NULL)
 		continue;
 	      if (sscanf (tmp, "%u", &size) != 1)
 		continue;
 
-	      tmp = strtok (NULL, " ");
+	      tmp = strtok_r (NULL, " ", &saveptr);
 	      if (tmp == NULL)
 		continue;
 	      if (sscanf (tmp, "%d", &uses) != 1)
 		continue;
 
-	      dependencies = strtok (NULL, " ");
+	      dependencies = strtok_r (NULL, " ", &saveptr);
 	      if (dependencies == NULL)
 		continue;
 
-	      status = strtok (NULL, " ");
+	      status = strtok_r (NULL, " ", &saveptr);
 	      if (status == NULL)
 		continue;
 
-	      tmp = strtok (NULL, "\n");
+	      tmp = strtok_r (NULL, "\n", &saveptr);
 	      if (tmp == NULL)
 		continue;
 	      if (sscanf (tmp, "%llx", &address) != 1)
diff --git a/gdb/remote.c b/gdb/remote.c
index 8ea52d3..1ac9013 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5169,7 +5169,8 @@
   else
     {
       char *copy = xstrdup (remote_support_xml + 13);
-      char *p = strtok (copy, ",");
+      char *saveptr;
+      char *p = strtok_r (copy, ",", &saveptr);
 
       do
 	{
@@ -5180,7 +5181,7 @@
 	      return;
 	    }
 	}
-      while ((p = strtok (NULL, ",")) != NULL);
+      while ((p = strtok_r (NULL, ",", &saveptr)) != NULL);
       xfree (copy);
 
       remote_support_xml = reconcat (remote_support_xml,
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 873fbaa..fc73b23 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -316,8 +316,10 @@
   if (data)
     {
       adi_stat_t adi_stat = get_adi_info (pid);
-      char *line;
-      for (line = strtok (data.get (), "\n"); line; line = strtok (NULL, "\n"))
+      char *saveptr;
+      for (char *line = strtok_r (data.get (), "\n", &saveptr);
+	   line;
+	   line = strtok_r (NULL, "\n", &saveptr))
         {
           ULONGEST addr, endaddr;
 
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index dc988df..3830faa 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -221,9 +221,10 @@
   /*  Add syscall to its groups.  */
   if (groups != NULL)
     {
-      for (char *group = strtok (groups, ",");
+      char *saveptr;
+      for (char *group = strtok_r (groups, ",", &saveptr);
 	   group != NULL;
-	   group = strtok (NULL, ","))
+	   group = strtok_r (NULL, ",", &saveptr))
 	syscall_group_add_syscall (syscalls_info, sysdesc, group);
     }
 }

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ief6138965a24398e5fc064598cd8f2abd3b5047c
Gerrit-Change-Number: 484
Gerrit-PatchSet: 2
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: merged

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

* [pushed] Use strtok_r instead of strtok
  2019-11-02 17:27 [review] Use strtok_r instead of strtok Christian Biesinger (Code Review)
                   ` (2 preceding siblings ...)
  2019-11-06 20:04 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-11-06 20:04 ` Sourceware to Gerrit sync (Code Review)
  3 siblings, 0 replies; 5+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-06 20:04 UTC (permalink / raw)
  To: Christian Biesinger, Pedro Alves, gdb-patches

The original change was created by Christian Biesinger.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/484
......................................................................

Use strtok_r instead of strtok

Improves threadsafety. This will be important when the patch series at
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/176
lands.

gdb/ChangeLog:

2019-11-06  Christian Biesinger  <cbiesinger@google.com>

	* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
	* mi/mi-main.c (output_cores): Likewise.
	* nat/linux-osdata.c (linux_xfer_osdata_cpus): Likewise.
	(linux_xfer_osdata_modules): Likewise.
	* remote.c (register_remote_support_xml): Likewise.
	* sparc64-tdep.c (adi_is_addr_mapped): Likewise.
	* xml-syscall.c (syscall_create_syscall_desc): Likewise.

gdb/gdbserver/ChangeLog:

2019-11-06  Christian Biesinger  <cbiesinger@google.com>

	* linux-x86-low.c (x86_linux_process_qsupported): Use strtok_r
	instead of strtok.
	* server.c (handle_query): Likewise.
	(captured_main): Likewise.

Change-Id: Ief6138965a24398e5fc064598cd8f2abd3b5047c
---
M gdb/ChangeLog
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/linux-x86-low.c
M gdb/gdbserver/server.c
M gdb/linux-tdep.c
M gdb/mi/mi-main.c
M gdb/nat/linux-osdata.c
M gdb/remote.c
M gdb/sparc64-tdep.c
M gdb/xml-syscall.c
10 files changed, 56 insertions(+), 27 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 048c2dd..f43d3a5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2019-11-06  Christian Biesinger  <cbiesinger@google.com>
+
+	* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
+	* mi/mi-main.c (output_cores): Likewise.
+	* nat/linux-osdata.c (linux_xfer_osdata_cpus): Likewise.
+	(linux_xfer_osdata_modules): Likewise.
+	* remote.c (register_remote_support_xml): Likewise.
+	* sparc64-tdep.c (adi_is_addr_mapped): Likewise.
+	* xml-syscall.c (syscall_create_syscall_desc): Likewise.
+
 2019-11-06  Tom Tromey  <tom@tromey.com>
 
 	* tui/tui-interp.c: Don't include readline.h.
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 35684db..9d0afaa 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,13 @@
+2019-11-06  Christian Biesinger  <cbiesinger@google.com>
+
+	* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
+	* mi/mi-main.c (output_cores): Likewise.
+	* nat/linux-osdata.c (linux_xfer_osdata_cpus): Likewise.
+	(linux_xfer_osdata_modules): Likewise.
+	* remote.c (register_remote_support_xml): Likewise.
+	* sparc64-tdep.c (adi_is_addr_mapped): Likewise.
+	* xml-syscall.c (syscall_create_syscall_desc): Likewise.
+
 2019-11-01  Christian Biesinger  <cbiesinger@google.com>
 
 	* configure: Regenerate.
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index cafff6b..54bd2a2 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -912,9 +912,11 @@
       if (startswith (feature, "xmlRegisters="))
 	{
 	  char *copy = xstrdup (feature + 13);
-	  char *p;
 
-	  for (p = strtok (copy, ","); p != NULL; p = strtok (NULL, ","))
+	  char *saveptr;
+	  for (char *p = strtok_r (copy, ",", &saveptr);
+	       p != NULL;
+	       p = strtok_r (NULL, ",", &saveptr))
 	    {
 	      if (strcmp (p, "i386") == 0)
 		{
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 59e8a55..c5f7176 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2268,9 +2268,10 @@
 
 	  /* Two passes, to avoid nested strtok calls in
 	     target_process_qsupported.  */
-	  for (p = strtok (p + 1, ";");
+	  char *saveptr;
+	  for (p = strtok_r (p + 1, ";", &saveptr);
 	       p != NULL;
-	       p = strtok (NULL, ";"))
+	       p = strtok_r (NULL, ";", &saveptr))
 	    {
 	      count++;
 	      qsupported = XRESIZEVEC (char *, qsupported, count);
@@ -3633,12 +3634,11 @@
 	}
       else if (startswith (*next_arg, "--disable-packet="))
 	{
-	  char *packets, *tok;
-
-	  packets = *next_arg += sizeof ("--disable-packet=") - 1;
-	  for (tok = strtok (packets, ",");
+	  char *packets = *next_arg += sizeof ("--disable-packet=") - 1;
+	  char *saveptr;
+	  for (char *tok = strtok_r (packets, ",", &saveptr);
 	       tok != NULL;
-	       tok = strtok (NULL, ","))
+	       tok = strtok_r (NULL, ",", &saveptr))
 	    {
 	      if (strcmp ("vCont", tok) == 0)
 		disable_packet_vCont = true;
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 567b01c..18cee91 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -839,9 +839,10 @@
 			   "      Size", "    Offset", "objfile");
 	    }
 
-	  for (line = strtok (map.get (), "\n");
+	  char *saveptr;
+	  for (line = strtok_r (map.get (), "\n", &saveptr);
 	       line;
-	       line = strtok (NULL, "\n"))
+	       line = strtok_r (NULL, "\n", &saveptr))
 	    {
 	      ULONGEST addr, endaddr, offset, inode;
 	      const char *permissions, *device, *mapping_filename;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 0e99fa3..c14897a 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -696,8 +696,9 @@
   ui_out_emit_list list_emitter (uiout, field_name);
   auto cores = make_unique_xstrdup (xcores);
   char *p = cores.get ();
+  char *saveptr;
 
-  for (p = strtok (p, ","); p;  p = strtok (NULL, ","))
+  for (p = strtok_r (p, ",", &saveptr); p;  p = strtok_r (NULL, ",", &saveptr))
     uiout->field_string (NULL, p);
 }
 
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 67f9f3a..84357e2 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -566,11 +566,12 @@
 	      char *key, *value;
 	      int i = 0;
 
-	      key = strtok (buf, ":");
+	      char *saveptr;
+	      key = strtok_r (buf, ":", &saveptr);
 	      if (key == NULL)
 		continue;
 
-	      value = strtok (NULL, ":");
+	      value = strtok_r (NULL, ":", &saveptr);
 	      if (value == NULL)
 		continue;
 
@@ -1216,36 +1217,36 @@
 	{
 	  if (fgets (buf, sizeof (buf), fp.get ()))
 	    {
-	      char *name, *dependencies, *status, *tmp;
+	      char *name, *dependencies, *status, *tmp, *saveptr;
 	      unsigned int size;
 	      unsigned long long address;
 	      int uses;
 
-	      name = strtok (buf, " ");
+	      name = strtok_r (buf, " ", &saveptr);
 	      if (name == NULL)
 		continue;
 
-	      tmp = strtok (NULL, " ");
+	      tmp = strtok_r (NULL, " ", &saveptr);
 	      if (tmp == NULL)
 		continue;
 	      if (sscanf (tmp, "%u", &size) != 1)
 		continue;
 
-	      tmp = strtok (NULL, " ");
+	      tmp = strtok_r (NULL, " ", &saveptr);
 	      if (tmp == NULL)
 		continue;
 	      if (sscanf (tmp, "%d", &uses) != 1)
 		continue;
 
-	      dependencies = strtok (NULL, " ");
+	      dependencies = strtok_r (NULL, " ", &saveptr);
 	      if (dependencies == NULL)
 		continue;
 
-	      status = strtok (NULL, " ");
+	      status = strtok_r (NULL, " ", &saveptr);
 	      if (status == NULL)
 		continue;
 
-	      tmp = strtok (NULL, "\n");
+	      tmp = strtok_r (NULL, "\n", &saveptr);
 	      if (tmp == NULL)
 		continue;
 	      if (sscanf (tmp, "%llx", &address) != 1)
diff --git a/gdb/remote.c b/gdb/remote.c
index 8ea52d3..1ac9013 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5169,7 +5169,8 @@
   else
     {
       char *copy = xstrdup (remote_support_xml + 13);
-      char *p = strtok (copy, ",");
+      char *saveptr;
+      char *p = strtok_r (copy, ",", &saveptr);
 
       do
 	{
@@ -5180,7 +5181,7 @@
 	      return;
 	    }
 	}
-      while ((p = strtok (NULL, ",")) != NULL);
+      while ((p = strtok_r (NULL, ",", &saveptr)) != NULL);
       xfree (copy);
 
       remote_support_xml = reconcat (remote_support_xml,
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 873fbaa..fc73b23 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -316,8 +316,10 @@
   if (data)
     {
       adi_stat_t adi_stat = get_adi_info (pid);
-      char *line;
-      for (line = strtok (data.get (), "\n"); line; line = strtok (NULL, "\n"))
+      char *saveptr;
+      for (char *line = strtok_r (data.get (), "\n", &saveptr);
+	   line;
+	   line = strtok_r (NULL, "\n", &saveptr))
         {
           ULONGEST addr, endaddr;
 
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index dc988df..3830faa 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -221,9 +221,10 @@
   /*  Add syscall to its groups.  */
   if (groups != NULL)
     {
-      for (char *group = strtok (groups, ",");
+      char *saveptr;
+      for (char *group = strtok_r (groups, ",", &saveptr);
 	   group != NULL;
-	   group = strtok (NULL, ","))
+	   group = strtok_r (NULL, ",", &saveptr))
 	syscall_group_add_syscall (syscalls_info, sysdesc, group);
     }
 }

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ief6138965a24398e5fc064598cd8f2abd3b5047c
Gerrit-Change-Number: 484
Gerrit-PatchSet: 2
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: newpatchset

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

end of thread, other threads:[~2019-11-06 20:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-02 17:27 [review] Use strtok_r instead of strtok Christian Biesinger (Code Review)
2019-11-06  0:46 ` Pedro Alves (Code Review)
2019-11-06 20:03 ` Christian Biesinger (Code Review)
2019-11-06 20:04 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-06 20:04 ` Sourceware to Gerrit sync (Code Review)

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