Unsafe code produced when returing std::string @ByRef

[GoogleCodeExporter]

What steps will reproduce the problem?

Wrapping a function "std::string StringTest()" (using @ByRef) produces the 
following code:

const char* rpointer;
rpointer = (const char*)pointer->StringTest().c_str();
if (rpointer != NULL) {
    r = e->NewStringUTF(rpointer);
}
return r;

This is unsafe, because the temporary std::string holding the result of 
StringTest() does not persist any longer than the calling statement.   So 
rpointer ends up being a pointer into deallocated memory.

Original issue reported on code.google.com by dark...@gmail.com on 19 Oct 2012 at 3:21

[GoogleCodeExporter]

Actually, I've added a new adapter for std::string, e.g.:
    native @StdString String StringTest();
And with this recent fix 
http://code.google.com/p/javacpp/source/detail?r=d2765542855984ad68ce04c9846dbb5
a083fc95e it generates code like this:

        StringAdapter radapter(StringTest());
        rpointer = radapter;
        if (rpointer != NULL) {
            r = e->NewStringUTF(rpointer);
        }

So the std::string gets copied (but hopefully the compiler is smart enough to 
do some return value optimization).

Thanks for pointing out this problem though. Maybe we should get rid of this 
`@ByRef String` hack and just use the adapter, what do you think?

Original comment by samuel.a...@gmail.com on 20 Oct 2012 at 1:55

• Changed state: Accepted

[GoogleCodeExporter]

Seems reasonable.

Though I've been using the attached patch (against 0.2), If anyone else needs a 
quick fix...

Original comment by dark...@gmail.com on 20 Oct 2012 at 2:15

Attachments:

• stringtemppatch.diff

[GoogleCodeExporter]

Ah yes, in this case we only need to support std::string, no need to try and 
figure out the type. I'll apply your patch, no need to break existing code for 
no good reasons, thanks!

Original comment by samuel.a...@gmail.com on 20 Oct 2012 at 2:31

• Changed state: Started

[GoogleCodeExporter]

I did it a bit differently, but it should do the same thing. Let me know if you 
encounter any issues with this change:
http://code.google.com/p/javacpp/source/detail?r=37228aa780eae24c64e8011763d4191
51b6ee6d1
thanks!

Original comment by samuel.a...@gmail.com on 21 Oct 2012 at 11:40

[GoogleCodeExporter]

I've included these changes in JavaCPP 0.3. Let me know if you still have some 
problems with that, thanks!

Original comment by samuel.a...@gmail.com on 5 Nov 2012 at 11:20

• Changed state: Fixed