3

Quite new to c / c++. I have a question about the below code:

char* string2char(String command){
    if (command.length() != 0) {
        char *p = const_cast<char*>(command.c_str());
        return p;
    }
}

void setup() {}

void loop() {
    String string1 = "Bob";
    char *string1Char = string2char(string1);
    String string2 = "Ross";
    char *string2Char = string2char(string2);
    Serial.println(string1Char);
    Serial.println(string2Char);
}

This basically outputs repeatedly:

Ross
Ross

I understand I'm failing to grasp the concept of how pointers are working here - would someone be able to explain it? And how would I alter this so that it could show:

Bob
Ross
Allen
  • 3,601
  • 10
  • 40
  • 59
  • 2
    @codekaizer I don't see how. I assume `String` here is equivalent to `std::string`. – Lundin May 18 '18 at 15:02
  • @codekaizer yeah im definitely assuming they are pointing to same address, but I don't understand why? – Allen May 18 '18 at 15:04
  • Are you sure this is the actual code you are using? Copy/paste it? Also, are you sure this code created the binary you are using? Did you clean the build and build again? – Lundin May 18 '18 at 15:05
  • 11
    Undefined behavior - you're passing `String` by value, and it's being destroyed at the end of the function. – Mark Ransom May 18 '18 at 15:07
  • .c_string() is returning a C string which is a (char *)... of course your implementation will crash and burn because you don't copy it out... – Grady Player May 18 '18 at 15:08
  • 1
    @MarkRansom Oh of course. Good catch, you should post it as the answer to the question. Unless there's a good canonical dupe of this somewhere. – Lundin May 18 '18 at 15:09
  • sorry im really new to this, how would I edit it so that it works as expected? – Allen May 18 '18 at 15:09
  • @Allen Hang on and someone will post the answer. Mark found the bug. – Lundin May 18 '18 at 15:10
  • 2
    Out of curiosity what is this the actual use case of this? Just learning pointers? – ricco19 May 18 '18 at 15:11
  • 3
    Why do you cast away constness? Don't do it. Also no need to check for length 0. – rustyx May 18 '18 at 15:12
  • @ricco19 I'm actually learning Arduino at the moment, and I'd like to construct strings to pass to a function, but it only takes char* as arguments. That's why I'm declaring the strings first and then changing them to char*. There's probably a better way but I'm super n00b at C++ (I come from a JS background) – Allen May 18 '18 at 15:14
  • No need to be sorry, the question is OK. But casting away constness and fumbling around with pointers as yo do here is not. To make your code work you can pass a reference instead of a copy: `char* string2char(String & command)...`, but be aware that this is very poor practice. – Jabberwocky May 18 '18 at 15:14
  • 1
    @Allen you should post another question like "How can I pass a `String` to a function that accepts only `char*`" – Jabberwocky May 18 '18 at 15:16
  • `char *p = const_cast(command.c_str());` Whyyyyy? – Lightness Races in Orbit May 18 '18 at 15:32
  • @Lundin thanks, but sometimes a Ninja comment is all I have time for. – Mark Ransom May 18 '18 at 17:29

4 Answers4

4

This function :

char* string2char(String command){
    if (command.length() != 0) {
        char *p = const_cast<char*>(command.c_str());
        return p;
    }
}

Does not make much sense, it takes string by value and returns pointer to its internal buffer, with cased away constnes(don't do it). You are getting some odd behaviour as you are returning values of object that already was destroyed, pass it by ref. Also I'm curious why you need to do all this stuff, can't you just pass:

Serial.println(string1.c_str());
Serial.println(string2.c_str());
Mateusz Wojtczak
  • 1,621
  • 1
  • 12
  • 28
  • I was getting confused by this snippet: https://coderwall.com/p/zfmwsg/arduino-string-to-char – Allen May 18 '18 at 15:24
  • `I don't know how it compiles std don't have String type but string` This is tagged `Arduino` which uses own `String` object, luckily it has `.c_str()` method too – Killzone Kid May 18 '18 at 16:10
3

As noted by Mark Ransom in the comments, when you pass the string by value, the string command is a local copy of the original string. Therefore you can't return a pointer to its c_str(), because that one points at the local copy, which will go out of scope when the function is done. So you get the same bug as described here: How to access a local variable from a different function using pointers?

A possible solution is to rewrite the function like this:

const char* string2char(const String& command){
  return command.c_str();
}

Now the string is passed by reference so that c_str() refers to the same string object as the one in the caller (string1). I also took the libery to fix const-correctness at the same time.

Please note that you cannot modify the string by the pointer returned by c_str()! So it is very important to keep this const.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • 1
    Presumably he wants `char*` in a modifiable state or else this function is utterly redundant to just using `c_str()` in place. At that point design becomes a question, why do you need a `char*`. – ricco19 May 18 '18 at 15:26
2

The problem here is that you've passed String command to the function by value, which makes a copy of whatever String you passed to the function. So, when you call const_cast<char*>(command.c_str()); you're making a pointer to the c string of that copied String. Since the String you've cast is within the scope of the function, the memory is freed when the function returns and the pointer is essentially invalid. What you want to do is change the argument to String & command which will pass a reference to the string, whose memory won't be freed when the function returns.

samuelnj
  • 1,627
  • 1
  • 10
  • 19
2

Your issue revolves around your argument.

char* string2char(String command){   
       // create a new string that's a copy of the thing you pass in, and call it command
    if (command.length() != 0) {
        char *p = const_cast<char*>(command.c_str());  
             // get the const char* that this string contains.  
             // It's valid only while the string command does; and is invalidated on changing the string.

        return p;   /// and destroy command - making p invalid
    }
}

There are 2 ways to resolve this. The first and most complex, is to pass command in by reference. Thus const String& command and then work with that.

The alternative, which is much simpler, is to completely delete your function; make your char* const char* and just call c_str() on the string; ie

String string1 = "Bob";
const char *string1Char = string1.c_str();
UKMonkey
  • 6,941
  • 3
  • 21
  • 30