Particle.publish() corrupts memory with too many characters in data

This is pretty strange.

I’m trying to publish an event when an RFID card is scanned. I want to tell the cloud what card was scanned so we can log it. And each card the system knows about is stored in a struct with the code and a greeting string.

But when I publish an event before the LCD screen prints out the greeting associated with a card, it seems to corrupt the string data in those greetings and I get nonsense on the LCD. If the data payload of the event has 3 or less characters, everything works as expected. But if the data payload has 4 or more characters it corrupts data.

void checkCode(int code) {
  // Everything works fine with this line
  Particle.publish("sentry/card-scanned", "123");

  // card[0].greeting gets changed to nonsense if I include this line
  // Particle.publish("sentry/card-scanned", "1234");

  lcd.clear();
  lcd.print(cards[0].greeting);
}

What is going on? How is it possible Particle.publish() could have side effects like this?

Hi @Squeegy

It sure sounds like memory is getting corrupted in your program. Can you post the entire thing? I don’t see anything in the part you posted but the declaration of cards and how you write it, would be particularly interesting to see.

Here’s my .ino https://gist.github.com/Squeegy/b2e41eb93e54152fa958

which includes the full implementation of my checkCode() function as well.

Card is a struct:

typedef struct {
  int rfid; // the code from the reader
  bool allow; // true if allowed in, false if not
  char* greeting; // Text that will be shown the card owner when scanned
} Card;

And I make an array of them like:

Card cards[MAX_CARDS];

This is only the pointer to the string.
Where is the actual memory the string 'greeting' is stored in?

2 Likes

It’s assigned here: https://gist.github.com/Squeegy/b2e41eb93e54152fa958#file-sentry-ino-L156

Parsed from a tab delimited CSV passed to the appendMembers event handler.

It’s quite possible I’m handling strings entirely wrong… They’ve given me quite a bit of trouble.

When you receive your token just storing away its location where it came in is not enough, since this space will be reused later on.
What you’d need to do is allocate some memory to store a copy of it and then strcpy() the contents into this space and store this location in your pointer variable.
Or instead of dynamically allocating memory and storing the pointer you could declare your greeting like this char greeting[MAX_GREETING_LEN]; and directly copy into this space.

1 Like

From what I can see, the pointer is derived from

  char* token = strtok((char*)data, delim);

some few lines above. I have never used 'strtok' and a quick look into its reference revelas this odd statement:

Each call to strtok() returns a pointer to a null-terminated string
containing the next token. This string does not include the
delimiting byte. If no more tokens are found, strtok() returns NULL.

Probably you'll have to append a '\0'.

Nope, that’s not how strtok() works.
It actually injects a '\0' in place of each delimiter and returns the start of that string and subsequently advances its internal pointer to the position directly after the just injected zero-terminator.
If no more delimiter is found it just returns the current position which will return the last token which was already zero-terminated when it came in.
So no need to add a '\0' manually.

1 Like

Thanks, that makes sense.

It only was working before because nothing was overwriting that memory location with other data. The fact that data was sticking around told me I did that stuff right, which that threw me off quite a bit!

I’m now declaring the greeting as char greeting[16] and doing a strcpy(card[i].greeting, token) in my parsing to save that greeting, which should get this where it won’t get overwritten.

Thank you the sanity check guys, I’ll get the hang of char* one of these days.

2 Likes

Safety first.

strncpy(card[i].greeting, token, sizeof(Card.greeting));
card[i].greeting[sizeof(Card.greeting) - 1] = ‘\0’;

Buffer overflows are evil.

2 Likes

Maybe you can simply make it more dynamic:

char* token = strtok((char*)data, delim);
 ...
char *buffer = malloc(strlen(token) + 1);
strcpy(buffer,token);
cards[0].greeting = buffer;