ESP8266 getting garbage data from char array

Question

I'm honestly at a loss here. I'm trying to store an SSID and password that the user sends through a post request in the flash EEPROM section. To do that I convert the data sent from the post request to a char array and index it to EEPROM. The SSID runs without any problems, but the password always ends up with junk data before it even gets to EEPROM. Here is the bit of code in question:

// Recieve data from the HTTP server
void changeConfig(String parameter, String value){
  int memoffset = 0;
  if(parameter == "ssid")
    memoffset = 0;
  else if(parameter == "pass")
    memoffset = 32;
  else
    return;
  #ifdef DEBUG
  Serial.println("Updating Data");
  Serial.print("Param: ");
  Serial.println(parameter);
  Serial.print("Value: ");
  Serial.println(value);
  #endif
  EEPROM.begin(64);
  char _data[sizeof(value)];
  value.toCharArray(_data, sizeof(value));
  for(int i = memoffset; i < memoffset + sizeof(value); i++)
  {
    #ifdef DEBUG
      Serial.print("addr ");
      Serial.print(i);
      Serial.print(" data ");
      Serial.println(_data[i]);
      #endif 
      EEPROM.write(i,_data[i]);
  }
  EEPROM.end();
}

And the Serial monitor output:
Post parameter: ssid, Value: NetworkName
Updating Data
Param: ssid
Value: NetworkName
addr 0 data N
addr 1 data e
addr 2 data t
addr 3 data w
addr 4 data o
addr 5 data r
addr 6 data k
addr 7 data N
addr 8 data a
addr 9 data m
addr 10 data e
addr 11 data ␀
Post parameter: pass, Value: Networkpass
Updating Data
Param: pass
Value: Networkpass
addr 32 data |
addr 33 data (
addr 34 data �
addr 35 data ?
addr 36 data L
addr 37 data ␛
addr 38 data �
addr 39 data ?
addr 40 data ␁
addr 41 data ␀
addr 42 data ␀
addr 43 data ␀

As you can see, when the name of the POST parameter is ssid, it works alright. With pass on the other hand, the char array is just filled with gibberish. Any insight would be helpful. I'm using platformio in the arduino environment. Generic ESP01 with 1M of flash. Thanks in advance.

Answer 1

You have two problems with your code.

First, you are using sizeof incorrectly. Sizeof returns the size of the String object, but you are trying to get the length of the contained string. Sizeof is not the right tool for that, instead you should use whatever API String offers to read the size of the string.

The next problem is your usage of offsets. The following code snippet is all wrong:

char _data[sizeof(value)];
value.toCharArray(_data, sizeof(value));
for(int i = memoffset; i < memoffset + sizeof(value); i++)
{
  ...
  EEPROM.write(i,_data[i]);

Your i starts with offset of 32, so you are trying to access element with index 32 in your _data array. But _data stores characters starting from the index 0, and since the length of array is actually 12 (sizeof of String is always 12) by accessing element with index 32 you are going beyond it's bounds, and obviously find garbage there (in C++ parlance, it is called undefined behavior).

Last, but not the least, C++ is an extremely complicated language, which can't be learned by 'trial and error'. Instead, you need to methodically study, preferably using one of the good C++ books. The list of those can be found here: The Definitive C++ Book Guide and List

Answer 2

You're using sizeof() incorrectly.

sizeof() tells you the size of the object, at compile time.

Try this experiment - run this code:

#include <Arduino.h>

void setup() {
  String x("");
  String y("abc");
  String z("abcdef");

  Serial.begin(115200);

  delay(1000);

  Serial.println(sizeof(x));
  Serial.println(sizeof(y));
  Serial.println(sizeof(z));
}

void loop() {
}

On my ESP8266 this outputs:

12
12
12

That's because it takes 12 bytes using this development environment to represent a String object (it might be different on a different CPU and compiler). The String class dynamically allocates storage, so sizeof can tell you nothing about how long the string itself is, only the compile-time size of the object.

For the String class, you should use its length() method. Your lines:

char _data[sizeof(value)];
value.toCharArray(_data, sizeof(value));
for(int i = memoffset; i < memoffset + sizeof(value); i++)

should be written as

char _data[value.length()];
value.toCharArray(_data, value.length());
for(int i = memoffset; i < memoffset + value.length(); i++)

For more information see the documentation on the String class.

You'll likely still have issues with string terminators. C and C++ terminate char array strings with the null character '\0', adding an extra byte to the length of the strings. So your code should more likely be:

void changeConfig(String parameter, String value){
  int memoffset = 0;
  if(parameter == "ssid")
    memoffset = 0;
  else if(parameter == "pass")
    memoffset = 33;
  else
    return;
  #ifdef DEBUG
  Serial.println("Updating Data");
  Serial.print("Param: ");
  Serial.println(parameter);
  Serial.print("Value: ");
  Serial.println(value);
  #endif
  EEPROM.begin(66);
  char _data[value.length() + 1];
  value.toCharArray(_data, value.length() + 1);
  for(int i = memoffset; i < memoffset + value.length() + 1; i++)
  {
    #ifdef DEBUG
      Serial.print("addr ");
      Serial.print(i);
      Serial.print(" data ");
      Serial.println(_data[i]);
      #endif 
      EEPROM.write(i,_data[i]);
  }
  EEPROM.end();
}

to allow the string terminators to work correctly for 32 character SSIDs and passwords. But the fundamental issue that's breaking your code is the incorrect use of sizeof.