0

I am trying to make a pointer to a constant character array from a c++ string. In the last four lines, I am adding the three strings to one string. This should be used to create a pointer to a constant array. This pointer should then be returned to be used in another function. When I am debugging step by step, the "cout" at the end of the function shows the correct behaviour. When I am looking at the returned value in the main function, it points to garbage data. What am I doing wrong while returning the pointer?

const char *checkMultiID(void){
    string startID = "USB0::0x2A8D::0x0101::";
    string usbID = "MY54500604";
    string endID = "::0::INSTR";
    char answerID;
    int correctFunctionInput = 0;

    cout << "ID = " << usbID << "? [Y/N]" << endl;
    scanf("%c", &answerID);

    while(correctFunctionInput == 0){
        if ((answerID == 'Y') || (answerID == 'N')){
            correctFunctionInput = 1;
        }
        else{
            cout << "Incorrect Input. Please repeat." << endl;
            scanf("%c", &answerID);
        }
    }

    if (answerID == 'N'){
        cout << "Please Type in the ID like MY..." << endl;
        getline (cin, usbID);
    }

    string fullID = startID + usbID + endID;
    const char *idChar = &fullID[0];
    cout << idChar << endl;

    return idChar;
}
genpfault
  • 51,148
  • 11
  • 85
  • 139
  • 4
    That's because the string goes out of scope at the end of the function, so the `char*` that's pointing to the string's data won't be valid anymore afterwards. – Blaze Sep 19 '19 at 14:15
  • 1
    Possible duplicate of [Can a local variable's memory be accessed outside its scope?](https://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope) – Algirdas Preidžius Sep 19 '19 at 14:17
  • So I would need to create a global string? –  Sep 19 '19 at 14:22
  • 1
    @Alex Just return the whole `std::string`. – François Andrieux Sep 19 '19 at 14:26
  • Okay, it is working with a global string. Are there other solutions I did not think of that don't require a global variable? –  Sep 19 '19 at 14:26
  • You could work with a global string or pass a char** to this function and set its value or even better just return the string and get its internal char* in the caller. – mcabreb Sep 19 '19 at 14:27
  • @FrançoisAndrieux ...and create the const char * where it is needed later on.. I could have thought of that, thanks. –  Sep 19 '19 at 14:27
  • @mcabreb Just taking a `char**` won't help. It doesn't solve the lifetime problem. – François Andrieux Sep 19 '19 at 14:27
  • @Alex Note that `std::string` has a convenient `c_str()` member to get a `const char *` to the string. – François Andrieux Sep 19 '19 at 14:29
  • @FrançoisAndrieux you can set a char* array in the caller with enough space and pass it by reference or pointer to checkMultiID and copy the final string contents to the received parameter. There would be no lifetime problem. Of course, it's a weird solution but I do not know the OP constraints. Returning a std::string is better. – mcabreb Sep 19 '19 at 14:31
  • @mcabreb But then you don't need `char**`. Just `char*` works. `char**` implies you want to modify the address points to or otherwise need access to the original pointer object. – François Andrieux Sep 19 '19 at 14:33
  • @FrançoisAndrieux yes, that's true. – mcabreb Sep 19 '19 at 16:04
  • Possible duplicate of [What exactly happens when returning const reference to a local object?](https://stackoverflow.com/questions/11747652/what-exactly-happens-when-returning-const-reference-to-a-local-object) – ead Sep 20 '19 at 06:26

2 Answers2

0

You are returning a pointer to data which is handled by a container, the c++ string, which is going out of scope, therefore is being deconstructed at the end of the function. What you want to do to get the exact behavior you describe is use a heap allocation, like so:

char* result = new char[fullID.length()+1];
std::copy(string.c_str(),string.c_str()+fullID.length()+1,result);

What you should to is return the c++ string directly, because I guarantee you that you will forget to deallocate this string eventually.

const string checkMultiID(){
  return fullID;
}
Wolfgang Brehm
  • 1,491
  • 18
  • 21
-2

You can add static keyword to fullID like :

static string fullID = startID + usbID + endID;

Better option would be to just return string.

Edit:

1201programalarm is right.

To avoid this you can do :

static string fullID;
fullID  = startID + usbID + endID;

In this case after second call of checkMultiID() value from the first call will be deleted.

If you change function return type to std::string then you can just return fullID and in another function call c_str() method of returned string.

const char * result = checkMultiID().c_str();

This will solve your problem and it's the simplest solution.

Zenek
  • 70
  • 5
  • 2
    Not a good solution. This will only build the string the first time, and will result in an incorrect returned string for subsequent calls that have different inputs. – 1201ProgramAlarm Sep 19 '19 at 14:33
  • Your "simplest solution" is not a solution to anything, it's always wrong. – Ben Voigt Sep 19 '19 at 16:34
  • `const char * result = checkMultiID().c_str();` also doesn't work. `result` becomes dangling as soon as the expression ends. It's unusable. You need to store the result into an actual `std::string`. – François Andrieux Sep 19 '19 at 17:30