Char corrupted by strtok

[tomparkin]

What exactly so you mean with "you must be careful to ensure that the scope of these arrays stays in sync"?

Sorry, that probably wasn't very clear. Scope is an important topic but is well-explained elsewhere (try a web search for "c and c++ scoping rules" for lots of information).

Specific to the code I posted, what I meant was the following.

In the PROGMEM version of the code, the wave_definitions array is declared on the stack in the function read_definitions. This means that when the function read_definitions returns, the array is no longer valid.

Moreover, the pointers stored in the wave_definitions array are pointers to another array which is also declared on the stack in read_definitions (the array tmp in the code I posted).

So what?

Well, just printing the strings in wave_definitions in the read_definitions function isn't massively useful: I assume you want to use those strings somewhere else in your code. You might think to make wave_definitions a global variable so that other parts of your code could access it after read_definitions returns.

But! The pointers it contain would be invalid since the tmp array "went out of scope" once read_definitions returned. This would likely lead to difficult-to-debug issues later on, and hours of terrible sadness.

So what I was really saying was that you need to ensure that the tmp array is valid for as long as you're accessing the pointers in the wave_definitions array. There are three ways to do this:

1. Make both tmp and wave_definitions global (or possibly pass them in from another function).
2. Do all your access to wave_definitions and the pointers it contains within read_definitions or a function that read_definitions calls.
3. Don't stack-allocate but instead dynamically allocate memory. Looking into this is an exercise for the reader

Hope this helps!

[apollo91]

Hey thanks a lot for clarifying! Now I think I understand it. Luckily I only call another function from read_definitions that extracts the values and writes them to global variables. After that there is no further use of the arrays so I think it should work that way

[apollo91]

Hey Tomparkin, I used your nice PROGMEM implementation and have an additional question and it would be great if you could help me another time. I now tried to list all the different definitions and and to pass a singe entry of the array to the read_definitions function. Unfortunately it does not work. Seems to be more complicated than that...

PGM_P const DEF_MODE_LIST[] PROGMEM =
{
    DEF_M_0,
    DEF_M_1,
    DEF_M_2,
    DEF_M_3
};

read_definitions(DEF_MODE_LIST[current_mode], strlen_P(DEF_MODE_LIST[current_mode])+1);

void read_definitions(const char *str, size_t len) {
  
  char *wave_definitions[MAX_NUM_WAVES] = {};
  char *mode_definitions;
  char tmp[len];
  byte wave_idx = 0;
  char *ptr;
 
  strncpy_P(tmp, str, len);
   Serial.println(tmp);

  ptr = strtok(tmp, "~");
  if (!ptr) return;

  mode_definitions = ptr;

  while((ptr = strtok(NULL, "~"))) {
    Serial.println(ptr);
    wave_definitions[wave_idx++] = ptr;
    if (wave_idx >= MAX_NUM_WAVES) break;
  }

...

[gfvalvo]

apollo91:

PGM_P const DEF_MODE_LIST[] PROGMEM =

{
    DEF_M_0,
    DEF_M_1,
    DEF_M_2,
    DEF_M_3
};

IMO, putting the pointers themselves in PROGMEM really isn't worth the bother of dealing with double access to flash it requires -- i.e. once to pull out the pointers and once to pull out the actual strings. You almost always get the biggest bang for the buck when you just put the strings themselves in PROGMEM.

As it is, you code is not doing the above-mentioned double access correctly. Just leave the pointer array in RAM.

[apollo91]

I put it in RAM and it worked perfectly! Thanks a lot!