Arduino Stack Exchange is a question and answer site for developers of open-source hardware and software that is compatible with Arduino. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

Good afternoon,

In a nutshell I'm having some difficulty with splitting and returning part of a string.

Essentially I'm working on the code for transmitting a password (24 character string) and command (string) in a single packet. However, the 'get_command' function is only returning the first character of the string (T) as opposed to the whole string (T1) and I'm not sure why. Any help would be greatly appreciated. Thanks in advance.

The entire sketch is below so you should be able to just drop it in and compile.

char *received_data = "KkV3vKvLgC4PdBZRs5kkKM86T1";
char *password = "KkV3vKvLgC4PdBZRs5kkKM86";

int authenticate_transmission(char *data) {
    if(strncmp(data, password, 24)) {
        Serial.println("no match");
        return false;
    } else {
        Serial.println("match");
        return true;
    }
}

char *get_command(char *data) {
    char command[3];
    strncpy(command, data + 24, 3);
    // Fine here
    Serial.println(command);
    return command;
}

void setup()
{
    Serial.begin(9600);
}

void loop() {
    // test received data - message is T1 at the end.
    if (authenticate_transmission(received_data)) {
        char *command = get_command(received_data);
        // But just prints out first char when returned from a function
        Serial.println(*command);
    }
    delay(10000);
}
share|improve this question
up vote 1 down vote accepted

The command array created inside get_command() is local data. That means it's only available while that function is being executed.

However, you're returning a pointer to it. That means when you're trying to use command afterwards from inside loop(), you're trying to refer to data which has been deallocated. By that point, it's getting used for something else.

There are various solutions to this. A fairly simple approach would be to have the caller allocate the command array, passing a pointer to it into get_command(). For example:

void get_command(char *data, char *command) {
    strncpy(command, data + 24, 3);
}

From loop(), you would call it like this:

char command[3];
get_command(received_data, command);

The command array is still local data in this case. However, it's local to the loop() function (or wherever else you put that code). That means it will still be available when get_command() is called.

share|improve this answer
    
Saying static char command[3] instead is slightly easier – jwpat7 Apr 28 '15 at 15:35
    
@jwpat7 That would work, but it's not good programming practice. – Peter R. Bloomfield Apr 28 '15 at 16:08
    
Thanks Peter, that's very helpful. I've got it all sorted now and a bit more insight. Thanks again. – oduffy Apr 29 '15 at 15:12

The direct issue is you have a dereference operator (*) in the loop that you don't need:

Serial.println(*command);

This causes it to pass only the first character to Serial.println() instead of the pointer to the memory location. That's the first issue.

Like Peter mentioned, the string gets deallocated when returned from the get_command() function. It's undefined behavior, but it's luck that it worked the way it did.

share|improve this answer
    
Good catch. Hadn't spotted that myself. – Peter R. Bloomfield Apr 28 '15 at 13:09
    
Thanks very much for your help – oduffy Apr 29 '15 at 15:12

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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