console method String or const Char* as output?

#1
The following two examples hopefully make it a little clearer.

DefineEngineFunction( myFunction, const char*, (),, "" )
{
   char buffer[ 256 ];
   return buffer; // BUG!  This is returning temporary storage.
}

DefineEngineFunction( myFunction, String, (),, "" )
{
   char buffer[ 256 ];
   return buffer; // No bug.  buffer is copied to a String and then to the TorqueScript VM's string stack.
}

In short, temporary strings need to be copied to the TorqueScript VM's evaluator stack via Con::getReturnBuffer whereas a more permanent string like a string property on a SimObject can be directly passed out and need not be copied.

Using String as a return type to indicate a temporary string is just a convenience. The above example isn't making good use of it because it requires an extra conversion to String before the return, but take something like this instead:

DefineEngineFunction( myFunction, String, (),, "" )
{
   return String::ToString( "%s_%s.png", "some", "stuff" );
}

#2 Thanks for the samples & explanation.
I think got your point.

I think I will stick to return type "String" for all cases.

except if there is an extra workload/steps compare to using getReturnBuffer ?

#3 in this sample does the 2nd getReturnBuffer is needed?

DefineEngineMethod(SimCurl, getURL, const char *, (),,
	"Set the url.\n"
	"@param url url value")
{ 
	char *buffer = Con::getReturnBuffer(512);
	dStrcpy(buffer, object->getURL());
	return Con::getReturnBuffer(buffer); 
}

#4

Quote:except if there is an extra workload/steps compare to using getReturnBuffer ?

If you go from a "const char*" that has sufficient lifetime to a String return, that's an extra step of copying the data to the String. In that case, it's more efficient to directly return the "const char*".

However, when you need getReturnBuffer anyways, using String is much more convenient and only slightly less performant.

When you have a String already, using String is the fastest option.

Quote:in this sample does the 2nd getReturnBuffer is needed?

Depends on the lifetime of SimCurl::getURL() but in all likelihood what's returned there by SimCurl::getURL() has a lifetime that's bound to the SimCurl object so it will definitely exceed the required lifetime to not cause errors in the TS VM's execution. Thus getReturnBuffer is not needed.

BTW, the implementation there with the fixed buffer is real bad. If getReturnBuffer is used, it should at least do something like:

DefineEngineMethod(SimCurl, getURL, const char *, (),,
	"Set the url.n"
	"@param url url value")
{
   const char* url = object->getURL();
   const U32 len = dStrlen( url ) + 1;
   char* buffer = Con::getReturnBuffer( len );
   dMemcpy( buffer, url, len );
   return buffer;
}

#5 Ok I get the temporary / lifetime thing.
In fact, when the method is linked to an object and you return char *, the value is linked to the lifetime of the object.

As per my understanding, the call for a method is synchronous and not asynchronous, so do we have really a risk to loose our information and bug/crash?
Or does that mean if I call the method that give me the Url in an %var, then I delete the object then my %var won't be usable?

For function not linked to an object, I perfectly understand your first sample now.

Thanks for the tip on the function, I need to look at those implementation that comes from resources.

I think from the previous comment, I should do:

DefineEngineMethod(SimCurl, getURL, const char *, (),,
	"Get the url.n"
	"@param url url value")
{ 
	return object->getURL();
}

const char * SimCurl::getURL()
{
	return url;
}

in fact in our case URL is "char url[256]", this is why getting a buffer of 512 and put the url in it will works without out of bound memory writing. I think it was your point.
by the way, your implementation is the safest one, if Url size is changed.

Thanks for your time on this!

#6

Quote:As per my understanding, the call for a method is synchronous and not asynchronous, so do we have really a risk to loose our information and bug/crash?

It's mainly about temporaries. Usually, other data will be copied soon enough in order for this to not ever be a problem.

Quote:Or does that mean if I call the method that give me the Url in an %var, then I delete the object then my %var won't be usable?

No. Variables (locals and globals) have their own storage to which their values are copied.

Quote:n fact in our case URL is "char url[256]", this is why getting a buffer of 512 and put the url in it will works without out of bound memory writing. I think it was your point.
by the way, your implementation is the safest one, if Url size is changed.

I'm of course not familiar with the code there so it may be totally justified, but in general, arbitrary limitations like fixed-size character buffers are *bad*. I know, much of the Torque legacy code is still littered with them, but they shouldn't be there. Fixed-size buffers are generally an indication of programmer laziness. And yeah... I've done it more than once in my own code :)

#7

Quote:I'm of course not familiar with the code there so it may be totally justified, but in general, arbitrary limitations like fixed-size character buffers are *bad*. I know, much of the Torque legacy code is still littered with them, but they shouldn't be there. Fixed-size buffers are generally an indication of programmer laziness. And yeah... I've done it more than once in my own code :)

Yeah, had my fair share of them. Now I use the String class for everything in my code. Much better to work with.

#8 Thanks for the clarifications.

for C++, I have some time doubts about things. I'm coming from .Net world and I don't always get/see C++ "bad" implementation.

Now I will try to keep that in mind when I see/write buffers :D

I appreciated the time you take to answer my post.