How to use malloc to return C string from function

Question

I am trying to return a C string from a function. The function is suppose to concatinate the 3 integers with commas and return the result as a char array however I'm getting garbage values. I'm assuming I'm not calling malloc correctly. Can someone advise on what's the problem?

using namespace std;

const char * createCommand(int p1, int p2, int p3){
    stringstream sstm;
    std::string comma = ",";
    sstm << p1 << comma << p2 << comma << p3;
    std::string str = sstm.str();
    const char *cstr = (const char *)malloc( (str.length()+1) * sizeof (char));

    cstr = str.c_str();
    return cstr;    
}

int main() {
    const char *cstr2 = createCommand(1,0,250); //I want to return "1,0,250"
    printf("char = %s\n",cstr2);
}

Is there a specific reason you *have* to return a c-string? You should definitely not be writing code like this in C++ unless you have a very good reason. — Xirema, Feb 10 '17 at 21:14
In C++ you should use `new` rather than `malloc` to allocate memory dynamically. — Barmar, Feb 10 '17 at 21:15
@NathanOliver You mean return std::string and convert it into c string in main? — Lightsout, Feb 10 '17 at 21:22
@bakalolo: yes, exactly. Have the function return a `std::string`, and then the caller can use `std::string::c_str()` if it actually needs a `char*` pointer. — Remy Lebeau, Feb 10 '17 at 21:33
@Xirema I found some code online that writes to a serial port using the write() command which only takes c strings I'm assuming. I'm open to doing it in a c++ way if i ever figure out how to write to serial port using c++. — Lightsout, Feb 10 '17 at 21:53
@bakalolo In that case, the solution I provided should work without you needing to do any `malloc` calls, or for that matter, any nonsense with direct management of dynamic memory. I couldn't tell you the correct "C++" way to write that code without knowing which code/library it is you're using. — Xirema, Feb 10 '17 at 21:55

score 2 · Accepted Answer · answered Feb 10 '17 at 21:20

Since the other two answers already gave responses to the tune of dealing with the literal problem, I'm going to instead advise on what I consider a pretty significant design flaw causing your problem: returning c-strings.

In the example code you're providing, the use of c-strings at all makes no sense. The following code will achieve what you intend to do with no difficulty or problematic code:

std::string createCommand(int p1, int p2, int p3){
    std::stringstream sstm;
    std::string comma = ",";
    sstm << p1 << comma << p2 << comma << p3;
    return sstm.str();
}

int main() {
    std::string command = createCommand(1,0,250); //I want to return "1,0,250"
    std::cout << "char = " << command << "\n";
}

Even if you're confined to using printf instead of the C++ iostreams library, this design is still better:

std::string createCommand(int p1, int p2, int p3){
    std::stringstream sstm;
    std::string comma = ",";
    sstm << p1 << comma << p2 << comma << p3;
    return sstm.str();
}

int main() {
    std::string command = createCommand(1,0,250); //I want to return "1,0,250"
    printf("char = %s\n", command.c_str());
}

And if you need the c-string passed to some older, C-based library, this design will still suffice. The point being, there's no reason to use malloc or interface with the underlying c-string representation except through the string itself.

So the reason you don't have to deal with malloc for 'command.c_str()' is because it's in the main method? — Lightsout, Feb 10 '17 at 21:33
No, it is because `c_str()` simply returns a pointer to memory that `std::string` owns. `std::string` will take care of freeing that memory for you. — Remy Lebeau, Feb 10 '17 at 21:34
@bakalolo The reason you don't need to deal with malloc is because the `std::string` object manages its own dynamic memory. There's no reason for you to do it yourself. — Xirema, Feb 10 '17 at 21:34

Sergey Kalinichenko · Answer 2 · 2017-02-10T21:30:17.590

1

Assignment operator, which works fine for std::string and other objects, cannot have an override for pointers. Therefore, the assignment

cstr = str.c_str();

leaks the memory that you have allocated, and replaces the pointer with the data from the string. Moreover, the pointer that your function returns now, points into memory that is invalidated upon exiting the function, creating an undefined behavior in addition to a leak.

To fix this problem, call std::strcpy(cstr, str.c_str()); Don't forget to call std::free on the result of the call. Edit: you should remove const from the return type of your createCommand function (WhozCraig, thank you for the comment).

Note: I assume that this is only an exercise in using malloc, that you know that using new[] is preferable, and that you wouldn't have to do any of the above if you could return std::string from the function.

edited Feb 10 '17 at 21:30

answered Feb 10 '17 at 21:14

Sergey Kalinichenko

714,442
84
1,110
1,523

I would make no assumptions. Too many "C++ Programmers" are C Programmers who think C++ is just "C with classes", or that the same design principles apply to both languages. – Xirema Feb 10 '17 at 21:23
@Xirema According to OP's profile, it does not look like he's got a lot of prior C experience, so my best guess is that this is a learning exercise. – Sergey Kalinichenko Feb 10 '17 at 21:25
One additional note: the OP's code shows `createCommand` as returning `const char*`, which makes no sense when returning a `malloc` result, since the caller, short of breaking const-ness, cannot subsequently `free` the resulting pointer. The changes in this answer are exactly what the OP needs, but in addition, the call should return `char*`, at which point the recommendation to remember `free()` by the caller becomes viable. – WhozCraig Feb 10 '17 at 21:26
@WhozCraig Why would they be unable to `free` the pointer? The `const` use in this context makes the data immutable, but has no impact on whether the pointer itself can be freed. Heck, even a `char const * const` has no restriction against being `delete`'d or `free`'d, since neither operation makes a change to the pointer object itself. – Xirema Feb 10 '17 at 21:29
@dasblinkenlight The `const` qualifier doesn't need to be removed from the method signature, it would only need to be removed from the declaration site of the variable `cstr`, so that `strcpy` or `std::copy` can be used. It could still be returned as `const char *`. – Xirema Feb 10 '17 at 21:32
@Xirema did you actually *try* that? [Something like this?](http://ideone.com/nbWzu8). – WhozCraig Feb 10 '17 at 21:34
1

@WhozCraig Oh, you know what? I mixed up the rules for `free` and `delete`. The behavior of `free` is only as I describe it in ANSI C, in C++, it behaves the way you described. [`delete` behaves exactly the way I described](http://ideone.com/ukJ1UM), but `free` freaks out if you pass it a `const` pointer. – Xirema Feb 10 '17 at 21:41
1

@Xirema I'm amazed that even compiled. That's one lax C compiler. I always thought they were good enough to catch that these days. `free`ing const pointers [has been asked on SO before](https://stackoverflow.com/questions/2819535/unable-to-free-const-pointers-in-c). – WhozCraig Feb 10 '17 at 21:42

score 0 · Answer 3 · answered Feb 10 '17 at 21:13

You'll need to copy the string with some form of strcpy, before returning the pointer.

const char * createCommand(int p1, int p2, int p3){
    stringstream sstm;
    std::string comma = ",";
    sstm << p1 << comma << p2 << comma << p3;
    std::string str = sstm.str();
    const char *cstr = (const char *)malloc( (str.length()+1) * sizeof (char));

    strcpy(cstr, str.c_str());
    return cstr;    
}

How to use malloc to return C string from function

3 Answers3