1

I have following function for my Arduino:

String readLine() {
  String received = "";
  char ch;
  while (myFile.available()) {
    ch = myFile.read();
    if (ch == '\n' or ch == '\r') {
      if (ch == '\r')
      {
        //get rid of the \n
        myFile.read();
      }
      return String(received);
    } else {
      received += ch;
    }
  }
  myFile.close();
  hasFile = false;
  return received;
}

This function is used to return a line from a text file on a SD card. Since I do have to do quite some things on the returned string, I would like to have it as cstr instead of string to increase performance. But I am not able to convert this function to one that returns a Cstr. I tried this but it does not work:

char* readLine() {
  char* received = "";
  char ch;
  while (myFile.available()) {
    ch = myFile.read();
    if (ch == '\n' or ch == '\r') {
      if (ch == '\r')
      {
        //get rid of the \n
        myFile.read();
      }
      return received;
    } else {
      received += ch;
    }
  }
  return received;
}

Currently I am converting it after the function returns it, by using c_str(). It works, but I dont like it and would like to return a Cstr in the first place.

How can this be done?

Thank you!

2 Answers 2

2

You cannot work with c-strings like with Strings. You need to use the standard c-string functions (like sprintf(), strcpy() and siblings) or directly access the char array.

This gives the following problems with your code:

  • See this line:

      char* received = "";
    

    Here you create a pointer to a constant empty string. But this will NOT create a buffer, where you can put data. If you want to define a buffer to put the data in, you need to define an array:

      char received[30] = "";
    

    where 30 is the size of the array and thus the buffer has space for 29 characters (30 minus the one character, that is needed for the terminating null character). Now you could use a counter variable to save the current position in the buffer and set the array elements to the received character:

      int position = 0;
      ...
      received[position] = ch;
      position++;
    

    Then, when you reached the end of the buffer or you received end of line, you would end the data in the buffer with the null character:

      if (ch == '\n' || ch == '\r' || position == 29) {
          if (ch == '\r')
          {
              //get rid of the \n
              myFile.read();
          }
          received[position] = '\0';
    
  • Now you have the second problem: When defining a variable inside the function, it will become invalid, when you exit the function. You try to return a pointer to char, which will point to a non-valid memory location. Thus you cannot simply return an array, that was created inside the function (outside of dynamic allocation). Here commonly an external buffer is provided to the function by reference as a pointer. The function will then write into that buffer and does not need to return it.

      //defining the array outside of the function, for example at global scope
      #define BUFFER_SIZE  30
      char received[BUFFER_SIZE] = "";
      ...
      void readLine(char *buffer, int size) {
          ...
          // Here you can use buffer as a simple char array
          buffer[position] = ch;
      }
    

    Here I also provide the size of the array as parameter, so that the function can make sure not to exceed that (because that will give you bad and difficult to troubleshoot problems). Then, when the function has run, the read data is now in the received character array. This way you don't need to use multiple buffers and also don't need to copy data between buffers.

1

One way is passing external buffer to readLine() to fill:

size_t readLine(char * buff, size_t maxChars) {
  size_t pos = 0; // current position in buffer
  while (file.myFileAvalable()) {
    buff[pos] = file.read();
    if (buff[pos] == '\n' || buff[pos] == '\r') {
      buff[pos] = '\0';   // terminate string
      break;              // end the loop
    }
    if (++pos == maxChars-1) { // don't forget about nul terminal character
      buff[pos] = '\0';
      break;              // end the loop
    }
  }
  return pos; // return number of read characters (0 nothing was read)
}

/// ...

char ssid[25] = {0}; // create buffer to store at most 24 characters + \0
char pass[25] = {0}; // it'll also fill buffers by zeroes

readLine(ssid, sizeof(ssid)-1);  // it'd be better to check how many characters was read
readLine(pass, sizeof(pass)-1);  // same here

WiFi.begin(ssid, pass);
/// ... But it looks much worst than String, however it's much more efficient, as using += for Strings is quite heap unfriendly 

Big NO-NO is doing something like:

char* readLine() {
  char buff[50] = {0};
  // ... fill ...
  return buff;           // NO!!!! buffer doesn't exist after the return - you've got dangling pointer
}
// ...
char * ssid = readLine(); // dangling pointer after that...
char * pass = readLine(); // now the ssid location was overwritten for sure

Another NO-NO:

char* readLine() {
  static char buff[50] = {0}; // one buffer for all calls, it won't get destroyed after the end... but
  // ... fill ...
  return buff;           // ok, buffer exists after the return,...
}
// ...
char * ssid = readLine(); // 
char * pass = readLine(); // but surprise surprise, ssid and pass points to the same buffer and therefore the same data...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.