Issues with strcpy and char arrays.

Hi!

I am trying to receive a string via serial containing a standard format for date code. "Time: 00:00:00 MM/DD/YYYY"

I can get this string into an array of char called buf[33];

Then I want to copy the second string containg the time using strtok() delimited by space. Then I want to copy the tkn (char *) returned by strtok() into ntpDate[8]. This method works for getting the date string into ntpDate[10] but for ntpDate I get an empty array or at least an array with a white space character only. I also Serial.println'ed the tkn and it appears to be correct before and after I use strcpy. Relevant code below. Any help is appreciated.

  String recieved = s;
  char buf[33];
  char ntpClock[8];
  char ntpDate[10];

  recieved.toCharArray(buf,33);
  char * tkn;

  //seperate into strings for clock and date
  tkn = strtok(buf, " ");
  tkn = strtok(NULL, " ");
  strcpy(ntpClock,tkn); //WHERE MY ISSUE IS 
  tkn = strtok(NULL, " ");
  strcpy(ntpDate,tkn); //WORKS AS EXPECTED
  tkn = strtok(NULL, " ");
  strcpy(ntpClock,tkn); //WHERE MY ISSUE IS

The time uses 8 characters. 8 characters plus a terminating NULL will not fit in an 8 element array.

The date uses 10 characters. 10 characters plus a terminating NULL will not fit in an 10 element array.

I tried making it larger and smaller to force an error if I could. Nothing happens. Its like a singlenul character is going from tkn to ntpTime but when tkn to ntpDate it works fine.

well you do need space for the trailing '\0'

try looking at this, there might be a idea in there for something biting you

char buffer1[] = "Time: HH:NN:SS MM/DD/YYYY"; // I don't have \r\n at the end!
char buffer2[] = "Time: HH:NN:SS MM/DD/YYYY\r\n"; // I have \r\n at the end!
char buffer3[] = "Time: HH:NN:SS MM/DD/YYYY\r\n"; // I have \r\n at the end!

char ntpClock[9];
char ntpDate[11];

void setup() {
  Serial.begin(115200);
  // How this works with Buffer1
  Serial.print("["); Serial.print(buffer1); Serial.println("]");
  char * tkn = strtok(buffer1, " ");
  Serial.print("["); Serial.print(tkn); Serial.println("]");
  tkn = strtok(NULL, " ");
  Serial.print("["); Serial.print(tkn); Serial.println("]");
  strcpy(ntpClock, tkn);
  tkn = strtok(NULL, " ");
  Serial.print("["); Serial.print(tkn); Serial.println("]");
  strcpy(ntpDate, tkn);

  Serial.print("ntpClock = ["); Serial.print(ntpClock); Serial.println("]");
  Serial.print("ntpDate = ["); Serial.print(ntpDate); Serial.println("]");


  // How this works with Buffer2
  Serial.print("["); Serial.print(buffer2); Serial.println("]");
  tkn = strtok(buffer2, " ");
  Serial.print("["); Serial.print(tkn); Serial.println("]");
  tkn = strtok(NULL, " ");
  Serial.print("["); Serial.print(tkn); Serial.println("]");
  strcpy(ntpClock, tkn);
  tkn = strtok(NULL, " ");
  Serial.print("["); Serial.print(tkn); Serial.print("] ==> "); Serial.print(strlen(tkn)); Serial.println(" characters.. Overflow!");
  strcpy(ntpDate, tkn); // THERE IS A BUG HERE

  Serial.print("ntpClock = ["); Serial.print(ntpClock); Serial.println("]");
  Serial.print("ntpDate = ["); Serial.print(ntpDate); Serial.println("]");

  // How this works with Buffer3 and strncpy()
  Serial.print("["); Serial.print(buffer3); Serial.println("]");
  tkn = strtok(buffer3, " ");
  Serial.print("["); Serial.print(tkn); Serial.println("]");
  tkn = strtok(NULL, " ");
  Serial.print("["); Serial.print(tkn); Serial.println("]");
  strcpy(ntpClock, tkn);
  tkn = strtok(NULL, " ");
  Serial.print("["); Serial.print(tkn); Serial.print("] ==> "); Serial.print(strlen(tkn)); Serial.println(" characters.. Overflow!");
  strncpy(ntpDate, tkn, sizeof(ntpDate) - 1); // Now we are being careful!
  ntpDate[sizeof(ntpDate) - 1] = '\0';

  Serial.print("ntpClock = ["); Serial.print(ntpClock); Serial.println("]");
  Serial.print("ntpDate = ["); Serial.print(ntpDate); Serial.println("]");
}

void loop() {}

So if my input is buf[] = "Time: HH:MM:SS MM/DD/YYYY GMT+01\r\n"
And I want ntpClock[9]= "HH:MM:SS" and ntpClock[11]=MM/DD/YYYY.

So I will need to do:

buf[] = "Time: HH:MM:SS MM/DD/YYYY GMT+01\r\n"; //Compiler adds '\0'

ntpClock[9]; //9 characters for time string + '\0'
ntpDate[11]; //Same as above but with date string

tkn = strtok(buf, " "); //tkn = "Time:"

tkn = strtok(NULL, " "); //tkn = "HH:MM:SS"
strcpy(ntpClock, tkn); //Copy tkn to ntpClock array
tkn = strtok(NULL, " "); //tkn = "MM/DD/YYYY" 
strncpy(ntpDate, tkn, sizeof(ntpDate) - 1); //Copy tkn to ntpDate leaving room for null char
ntpDate[sizeof(ntpDate) - 1] = '\0';

To be honest if you know the structure of the buffer, and that numbers Are exactly where you expect them then you know the index and could just grab them directly from the array.

strtok is the wrong tool for the job if you know exactly where the characters are.

  // 0....+....1....+....2....+....3....+....
  // Time: 00:00:00 MM/DD/YYYY

  char buf[33];
  char ntpClock[9];
  char ntpDate[11];

  recieved.toCharArray(buf,33);
  memcpy(ntpClock, buf+6, 8)
  ntpClock[8] = '\0';  
  memcpy(ntpDate, buf+15, 10)
  ntpClock[10] = '\0';

If you are happy to overwrite the contents of buf, if you don't need the whole thing again, then it's even easier. Put string terminators into buf, and use it in place.

  // 0....+....1....+....2....+....3....+....
  // Time: 00:00:00 MM/DD/YYYY
  char buf[33];

  char *ntpClock = buf+6;
  char *ntpDate = buf+15;

  recieved.toCharArray(buf,33);
  buf[14] = '\0';
  buf[25] = '\0';

And alternative way of doing this is with a union. People used to do this kind of thing all the time with COBOL data sections. It's handy if you are working with a lot of variables all having the same fixed format.

union TimeDate {
  char[33] buf;
  struct Parsed {
    char scrap1[6];
    char time[8];
    char scrap2[1];
    char date[10];
  } parsed;
};

  TimeDate timeDate; 
  char ntpClock[sizeof(timeDate.parsed.time) + 1];
  char ntpDate[sizeof(timeDate.parsed.date) + 1];

  recieved.toCharArray(timeDate.buf,33);
  memcpy(ntpClock, timeDate.parsed.time, sizeof(timeDate.parsed.time));
  ntpClock[sizeof(timeDate.parsed.time)] = '\0';
  memcpy(ntpDate, timeDate.parsed.time, sizeof(timeDate.parsed.date));
  ntpDate[sizeof(timeDate.parsed.date)] = '\0';

@PaulMurray - re the union approach in C++

Note that in C++ this is against the standard but supported by most compilers. If you initialize the union with a buf, then accessing the parsed structure is implementation dépendant and undefined behavior.

The union is only as big as necessary to hold its largest data member. The other data members are allocated in the same bytes as part of that largest member. The details of that allocation are implementation-defined, and it's undefined behavior to read from the member of the union that wasn't most recently written. Many compilers implement, as a non-standard language extension, the ability to read inactive members of a union.

With the union approach don't forget to put the scrap values to '\0' to be able to use the arrays as C strings though which would then make the structure the latest member to be written to and thus legit to access, but with no standard guarantee that the former data will be still arranged the way you expected it to be.