The problem with subStr is that you ultimately return a pointer (sub) that points into a buffer (copy) that has ceased to exist. Since subStr has exited, the space its local variables once occupied has been returned to the system. This is an insidious error, because the return pointer points to data that was once (and may still be) valid.
A simple workaround may simply be to declare copy static. Or you could rewrite subStr to require that callers pass in their own "copy" buffer.
Ah, I see. The "delim" parameter for strtok_r is supposed to be a pointer to "a NULL-terminated set of characters", whereas you are passing a pointer to a single (non-terminated) character.
To fix, change subStr's second parameter to "char *delim", remove the "&" in the strtok_r call, and when you call subStr, use " " instead of ' '.
I think it's funny that we only get a "reentrant strtok()" on this platform, because without threads, reentrancy issues are exceedingly rare. Doing strtok() in ISRs and non-ISRs together is similarly unlikely.
If you want to develop safe string handling in C, you need to adopt the point of view that the caller owns the storage, the callee just manipulates it (and never goes outside the buffer limits expressed to it). No "static buffers" inside the string routines, just in the callers. It's possible to get the behavior right in other ways, but it's also possible to go bald from pulling out your hair.
And as mikalhart observes, carefully consider the differences between buffers (block of memory), strings (zero-terminated), and characters.
That was it! Not sure why I didn't realize this by looking at the ref pages! I guess this would allow you to define a string of several delimiters to tokenize with, pretty powerful.
The following now works as expected, thanks again!
// strtok test */
#include <string.h>
#define MAX_STRING_LEN 20
char *record1 = "one two three";
char *record2 = "Hello there friend";
char *p, *i;
void setup() {
Serial.begin(9600);
Serial.println("Split record1: ");
Serial.println(subStr(record1, " ", 1));
Serial.println(subStr(record1, " ", 2));
Serial.println(subStr(record1, " ", 3));
Serial.println("Split record2: ");
Serial.println(subStr(record2, " ", 1));
Serial.println(subStr(record2, " ", 2));
Serial.println(subStr(record2, " ", 3));
}
void loop () {
}
// Function to return a substring defined by a delimiter at an index
char* subStr (char* str, char *delim, int index) {
char *act, *sub, *ptr;
static char copy[MAX_STRING_LEN];
int i;
// Since strtok consumes the first arg, make a copy
strcpy(copy, str);
for (i = 1, act = copy; i <= index; i++, act = NULL) {
//Serial.print(".");
sub = strtok_r(act, delim, &ptr);
if (sub == NULL) break;
}
return sub;
}
output:
Split record1:
one
two
three
Split record2:
Hello
there
friend
I think it's funny that we only get a "reentrant strtok()" on this platform, because without threads, reentrancy issues are exceedingly rare.
I thought that was funny too, Halley. Even funnier is the fact that the solution I proposed defeats that re-entrancy protection by creating the static shared buffer in subStr.
@Xtalker: Halley is quite right in observing that next time you should move "copy" out of subStr and pass the caller-owned buffer in as a parameter to subStr. That way you could, for example, maintain buffers in two different places at once.
