strcmp doesn't seem to work

Question

I'm trying to compare a char array that I'm assembling to a string and I'm having trouble comparing the strings.

I'm getting the data for the char array as a set of bytes and I'm compiling it into a char array like this:

void messageHandler(char *topic, byte *payload, unsigned int length)
{
  char buffer[length];
  for (int i = 0; i < length; i++)
  {
    buffer[i] = (char)payload[i];
  }

So grabbing each of the ASCII codes and converting it into a character as I add it to the buffer.

After I do that I'm trying to print the buffer to the monitor and then compare it to a string. Though it's not working:

I know I'm getting all of those extra characters in the println b/c the buffer doesn't end in 0 so print keeps reading out memory bytes till it hits the zero, but I don't know why strcmp isn't showing the buffer matching the string "ravenclaw".

I'm sure it's for the same "missing the null terminator" issue, but I'm not sure how to fix it. Help is appreciated!!

Either I don't understand your question or you're on the verge of answering it yourself. — timemage, Dec 29 '20 at 23:31
It would be better if you put the code into your question as text, not a as a screen capture. — timemage, Dec 29 '20 at 23:42

timemage · Accepted Answer · 2020-12-30T00:18:44.567

You seem to already understand that it's not null-terminated and that's why it's not working the way you expect. So, I'll take a different tact for "how to fix it".

You can simply not treat the payload as a string, and instead use memcmp, as in:

if (length == 9 && memcmp(payload, "ravenclaw", 9) == 0) {

memcmp behaves similarly to strcmp, except that it isn't looking for a null terminator. It just compares however many bytes (chars) you specify. In practice you may want to use a variable to hold "ravenclaw" and to strlen() on it rather than specifying the length separately as I've done above, e.g.:

static const char ravenclaw[] = "ravenclaw";
static const size_t ravenclaw_len = sizeof ravenclaw - 1;
if (length == ravenclaw_len && memcmp(payload, ravenclaw, ravenclaw_len) == 0) {

or any number of variations on that which might use strlen(), or a pointer rather than array (if not using sizeof) or PROGMEM with strlen_P() and memcmp_P() or... etc, etc, possibly putting this into a function so you can reuse it.

As a side note, if you know payload contains non-null-terminated text that you want to print, you can use the overload of write() that allows you to specify the length.

Serial.write(payload, length);

Ah awesome, thanks for the extra detail. I wasn't aware of `memcmp`. That def seems like a better way of doing the comparison. — Chris Schmitz, Dec 30 '20 at 04:23

score 2 · Answer 2 · answered Dec 30 '20 at 02:38

2

The easier way to simply the code is to use strcpy to create a buffer that is 1 byte longer than the length and then terminate it with \0.

void messageHandler(char *topic, byte *payload, unsigned int length) {
  char buffer[length+1];
  strcpy(buffer, (const char*) payload);
  buffer[length] = '\0';

  Serial.println(buffer);

  int matched = strcmp(buffer, "ravenclaw");
}

answered Dec 30 '20 at 02:38

hcheung

1,149
6
12

oh nice! I like this take on it. TBH I thought I tried this at one point and it didn't work, though I was noodling a lot at the time so I may have jacked up my code. thanks for the tip! – Chris Schmitz Dec 30 '20 at 04:24
Ah, the snag in this one is that after casting to a const char pointer you can't add anything to it. You end up with a `invalid conversion from 'const char*' to 'char', buffer[length] = "\0"` error. – Chris Schmitz Dec 30 '20 at 04:30
@ChrisSchmitz, what you seem to have missed is that in their code they used `'\0'` rather than `"\0"`, single rather than double quote. The terminator is a single character being written into a single character of an array of characters. It *should* compile any Arduino using g++, which is to say all of them that I know of; the run-time-sized local array is making use of a g++ extension. – timemage Jan 05 '21 at 04:47
@ChrisSchmitz However, if you want to try this way of doing it, **you need to swap out the `strcpy()` line and replace it with `memcpy(buffer, payload, length);`** Otherwise the `strcpy` may overflow `buffer`, because the payload is, again, not terminated. So there's nothing to tell `strcpy` to stop, except the next `'\0'` it finds in memory following `payload`. – timemage Jan 05 '21 at 04:48

strcmp doesn't seem to work

2 Answers2