diff mbox

[LEDE-DEV] ubox: Replace strtok with strsep

Message ID 20161218012222.24736-1-rosenp@gmail.com
State Changes Requested
Headers show

Commit Message

Rosen Penev Dec. 18, 2016, 1:22 a.m. UTC
Thread safe replacement.

Signed-off by: Rosen Penev <rosenp@gmail.com>
---
 kmodloader.c        | 8 ++++----
 lsbloader.c         | 6 +++---
 validate/validate.c | 2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

Comments

Felix Fietkau Dec. 18, 2016, 11:31 a.m. UTC | #1
On 2016-12-18 02:22, Rosen Penev wrote:
> Thread safe replacement.
> 
> Signed-off by: Rosen Penev <rosenp@gmail.com>
For ubox this change isn't really necessary. There's no thread safety
issues, or issues with use of strtok by callers.

- Felix
Arjen de Korte Dec. 18, 2016, 2:03 p.m. UTC | #2
Citeren Felix Fietkau <nbd@nbd.name>:

> On 2016-12-18 02:22, Rosen Penev wrote:
>> Thread safe replacement.
>>
>> Signed-off by: Rosen Penev <rosenp@gmail.com>
> For ubox this change isn't really necessary. There's no thread safety
> issues, or issues with use of strtok by callers.

I have my doubts about using strsep() on dynamically allocated  
buffers. In the scan_loaded_modules function of kmodloader.c, you'll  
lose the pointer to buf so you can't free the memory that was  
allocated by getline. Great, the function is now thread safe, but  
we've got a memory leak instead.

I would suggest to use strtok_r in cases where thread safety is an  
issue, rather than replacing it with strsep.
diff mbox

Patch

diff --git a/kmodloader.c b/kmodloader.c
index f80835a..65e7001 100644
--- a/kmodloader.c
+++ b/kmodloader.c
@@ -255,10 +255,10 @@  static int scan_loaded_modules(void)
 		struct module m;
 		struct module *n;
 
-		m.name = strtok(buf, " ");
-		m.size = atoi(strtok(NULL, " "));
-		m.usage = atoi(strtok(NULL, " "));
-		m.depends = strtok(NULL, " ");
+		m.name = strsep(&buf, " ");
+		m.size = atoi(strsep(&buf, " "));
+		m.usage = atoi(strsep(&buf, " "));
+		m.depends = strsep(&buf, " ");
 
 		if (!m.name || !m.depends)
 			continue;
diff --git a/lsbloader.c b/lsbloader.c
index b40a505..af9af3a 100644
--- a/lsbloader.c
+++ b/lsbloader.c
@@ -12,7 +12,7 @@ 
  *   along with this program; if not, write to the Free Software
  *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307, USA.
  *
- *   Copyright (C) 2012 John Crispin <blogic@openwrt.org> 
+ *   Copyright (C) 2012 John Crispin <blogic@openwrt.org>
  */
 
 #include <stdio.h>
@@ -148,10 +148,10 @@  static int init_services(void)
 		instances = blobmsg_open_table(&b, "instances");
 		instance = blobmsg_open_table(&b, "instance");
 		command = blobmsg_open_array(&b, "command");
-		t = strtok(i->exec, " ");
+		t = strsep(&i->exec, " ");
 		while (t) {
 			blobmsg_add_string(&b, NULL, t);
-			t = strtok(NULL, " ");
+			t = strsep(&i->exec, " ");
 		}
 		blobmsg_close_array(&b, command);
 		blobmsg_close_table(&b, instance);
diff --git a/validate/validate.c b/validate/validate.c
index e72b811..e15b886 100644
--- a/validate/validate.c
+++ b/validate/validate.c
@@ -178,7 +178,7 @@  dt_type_list(struct dt_state *s, int nargs)
 		return false;
 	}
 
-	for (p = strtok(str, " \t"); p; p = strtok(NULL, " \t"))
+	for (p = strsep(&str, " \t"); p; p = strsep(&str, " \t"))
 	{
 		s->value = p;