Why can't I return a value made with sprintf?

Hi. I ran into something unexpected when trying to return a value generated with sprintf(). I think it'd be easiest just to show the code and the output:

Example code:

void loop() {
    char* chan = "foo";
    char* subchan = "bar";
    char* full_name = full_chan_name(chan, subchan);
    Serial.print("from loop:   ");
    Serial.println(full_name);
}

char* full_chan_name(char* chan, char* subchan) {
    char* MY_MQTT_TOPIC = "plth";
    char combinedArray[sizeof(MY_MQTT_TOPIC) + sizeof(chan) + sizeof(subchan) + 8];
    sprintf(combinedArray, "%s/%s/%s", MY_MQTT_TOPIC, chan, subchan);
    Serial.print("from func:   ");
    Serial.println(combinedArray);
    return combinedArray;
}

Example output:

from func:   plth/foo/bar
from loop:   
from func:   plth/foo/bar
from loop:   
from func:   plth/foo/bar
from loop:   
from func:   plth/foo/bar
from loop:

What might be going on here? If it makes a difference, I'm using an off-brand ESP32 board with the Arduino IDE.

combinedArray goes out of scope after the function in which it is declared is exited.
Any pointer to it afterwards could return invalid results.

Thank you.

What makes it do that? How am I supposed to return values created by sprintf, or am I to put it in global scope? Also, is this a problem with sprintf specifically, or something else?

I had thought that returning a value would, you know, return the value, like it seems to do in every other case.

You can also add the static keyword before the declaration or, as you have said, make it global.

Edit:

ipsod:
I had thought that returning a value would, you know, return the value, like it seems to do in every other case.

The 'value' you return is a pointer to a data structure (array) which, in the mean time, has disappeared.

Thank you. It's an unexpected behavior. I guess I need to spend some time studying C, instead just winging it with Python/PHP experience.

It may not be intuitive, but it is well documented. Automatic variables (those defined within a function) have a limited lifetime and are invalid as soon as they go out of scope.

I guess what surprises me most is that I'm just now learning it - I'm sure I've written over 10k lines of code for Arduino, by now.

Is there some particular source you'd recommend for getting up to speed with this type of thing?

ipsod:
Thank you.

What makes it do that? How am I supposed to return values created by sprintf, or am I to put it in global scope? Also, is this a problem with sprintf specifically, or something else?

I had thought that returning a value would, you know, return the value, like it seems to do in every other case.

It did return a value. It returned the pointer just like you told it to. You didn’t tell it to return the array. You CAN’T tell it to return the array. You can only return a pointer and that’s what you got, a pointer pointing to the place in memory where that array was.

But when the function exits and the array goes out of scope, the compiler has no way to know what you plan to do with that pointer and it happily starts reusing the memory where that array used to be. The function is over and there’s no more reference to that array so it’s safe to destroy. But now your pointer, which is still the same value and still points to the same place, points to garbage.

Think of it like your friend gives you his address and asks you to come see him and then his house burns down. You still have a valid address but the house isn’t there anymore.

You’ll run into this same problem anytime you try to return a pointer to some local variable. That’s a well known and documented programming mistake.

ipsod:
Is there some particular source you'd recommend for getting up to speed with this type of thing?

Scope and lifetime of variables in a block structured language is not unique to C/C++.
Passing arrays around to/from functions implicitly as pointers is though more typical of such a low level language.
I guess you have to get an intermediate/advanced C/C++ reference book and work your way through it.

I bought this one some time ago. Jumping into C++, by Alex Allain - Cprogramming.com . I found it nicely structured because he does two iterations through the language. One at an intermediate level in the earlier chapters and one at an advanced level later on. I'm sure that there are also many other good options available.

6v6gt:
combinedArray goes out of scope after the function in which it is declared is exited.
Any pointer to it afterwards could return invalid results.

As a finer point on terminology (and you allude to this in a another post in this thread), it not only goes out of scope, but out of existence (its lifetime ends). If you make it static, it still goes out of scope when the function exits (you can't reference it by name from outside the function). But it still exists (lifetime has not ended). So, you can access it using the returned pointer.

OP, depending on the application, many times a better solution is not to return anything. Rather, create the data structure in the calling program and pass a pointer to it INTO the function to be manipulated there.

Also, local static variables may cause you a problem if you write a recursive function -- not too common in Arduino Land.

6v6gt:
I bought this one some time ago. Jumping into C++, by Alex Allain - Cprogramming.com . I found it nicely structured because he does two iterations through the language. One at an intermediate level in the earlier chapters and one at an advanced level later on. I'm sure that there are also many other good options available.

Thanks, I've been looking for a good book on advanced C++. How well does it cover creating templates? I can do basic ones, but when it comes to Template Specialization and Recursive Templates (e.g. varidic) the syntax is daunting.

Delta_G:
It did return a value. It returned the pointer just like you told it to. You didn’t tell it to return the array. You CAN’T tell it to return the array. You can only return a pointer and that’s what you got, a pointer pointing to the place in memory where that array was.

But when the function exits and the array goes out of scope, the compiler has no way to know what you plan to do with that pointer and it happily starts reusing the memory where that array used to be. The function is over and there’s no more reference to that array so it’s safe to destroy. But now your pointer, which is still the same value and still points to the same place, points to garbage.

Think of it like your friend gives you his address and asks you to come see him and then his house burns down. You still have a valid address but the house isn’t there anymore.

You’ll run into this same problem anytime you try to return a pointer to some local variable. That’s a well known and documented programming mistake.

Thank you, that makes sense. I just can't believe I've never yet tried to return a local variable and had it bite me. Does it behave differently for simpler data-types, like integers, floats, etc.?

gfvalvo:
As a finer point on terminology (and you allude to this in a another post in this thread), it not only goes out of scope, but out of existence (its lifetime ends). If you make it static, it still goes out of scope when the function exits (you can't reference it by name from outside the function). But it still exists (lifetime has not ended). So, you can access it using the returned pointer.

OP, depending on the application, many times a better solution is not to return anything. Rather, create the data structure in the calling program and pass a pointer to it INTO the function to be manipulated there.

Also, local static variables may cause you a problem if you write a recursive function -- not too common in Arduino Land.

Thank you. I'll do it that way, since it make sense, and could help me avoid problems down the road.

6v6gt:
Scope and lifetime of variables in a block structured language is not unique to C/C++.
Passing arrays around to/from functions implicitly as pointers is though more typical of such a low level language.
I guess you have to get an intermediate/advanced C/C++ reference book and work your way through it.

I bought this one some time ago. Jumping into C++, by Alex Allain - Cprogramming.com . I found it nicely structured because he does two iterations through the language. One at an intermediate level in the earlier chapters and one at an advanced level later on. I'm sure that there are also many other good options available.

Thanks, I've put the book on my list.

Thank you, that makes sense. I just can't believe I've never yet tried to return a local variable and had it bite me. Does it behave differently for simpler data-types, like integers, floats, etc.?

You have probably returned plenty of local variables. And what gets returned is the value that is in that variable. What you haven't yet done is return a POINTER to a local variable.

When you write this:

int myFunc(){

   int retVal = 5;
   return retval;
}

It's not the variable that gets returned, but the value. A copy of the value to be exact. So the number 5 gets returned.

When you do this:

int* myFunc(){

   int retval = 5;
   int* retptr = &retval;

   return retptr;
}

What gets returned is a copy of the value in retptr. That's just a memory address where the 5 is stored, but it isn't the 5 itself that is returned.

So when you get that pointer value back it points to a memory location. The same memory location it has always pointed to. The value of the POINTER is still intact. But the 5 has disappeared from that location. So when you dereference the pointer you find garbage.

The difference you have to see is the difference between returning a value and returning a pointer. Returning a value works like you want it to. Returning a pointer does not. You get a value that is just an address, but no guarantee that anyone still lives there.

Delta_G:
The difference you have to see is the difference between returning a value and returning a pointer. Returning a value works like you want it to. Returning a pointer does not. You get a value that is just an address, but no guarantee that anyone still lives there.

Ok, got it, thank you!

gfvalvo:
Thanks, I've been looking for a good book on advanced C++. How well does it cover creating templates? I can do basic ones, but when it comes to Template Specialization and Recursive Templates (e.g. varidic) the syntax is daunting.

Templates and template classes are covered but not variadic and specializations. Anyway, if you are going down to that arcane level then you have clearly passed the stage of needing a structured text book and are best served searching for guru level articles on such specific topics.

Another mistake:

  char* MY_MQTT_TOPIC = "plth";
  char combinedArray[sizeof(MY_MQTT_TOPIC) + sizeof(chan) + sizeof(subchan) + 8];

The sizes of the pointers 'MY_MQTT_TOPIC', 'chan' and 'subchan' are always 2, even though the pointers point to character strings of various lengths.

More accurate would be:

  char* MY_MQTT_TOPIC = "plth";
  // Allocate room for the strings plus two slashes and a terminating null.
  char combinedArray[strlen(MY_MQTT_TOPIC) + strlen(chan) + strlen(subchan) + 3];

You still get the warning:

sketch_aug13a.ino: In function 'char* full_chan_name(const char*, const char*)':
sketch_aug13a.ino:13:8: warning: address of local variable 'combinedArray' returned [-Wreturn-local-addr]
   char combinedArray[strlen(MY_MQTT_TOPIC) + strlen(chan) + strlen(subchan) + 3];

You can't use the 'static' keyword or you get the error:

sketch_aug13a.ino: In function 'char* full_chan_name(const char*, const char*)':
sketch_aug13a:13:87: error: storage size of 'combinedArray' isn't constant
   static char combinedArray[strlen(MY_MQTT_TOPIC) + strlen(chan) + strlen(subchan) + 3];
                                                                                       ^

I think your best bet is to pass a buffer:

void setup()
{
  Serial.begin(115200);
  while (!Serial);
  
  char chan[] = "foo";
  char subchan[] = "bar";
  char full_name[20];
  
  full_chan_name(full_name, sizeof full_name, chan, subchan);
  
  Serial.print("from setup:   ");
  Serial.println(full_name);
}


void full_chan_name(char *buffer, size_t bufflen, const char* chan, const char* subchan)
{
  const char MY_MQTT_TOPIC[] = "plth";
  
    // Make sure there is room for the strings plus two slashes and a terminating null.
  if ((strlen(MY_MQTT_TOPIC) + strlen(chan) + strlen(subchan) + 3) > bufflen)
  {
    strcpy(buffer, "OVERFLOW");
    return;
  }


  sprintf(buffer, "%s/%s/%s", MY_MQTT_TOPIC, chan, subchan);
  Serial.print("from func:   ");
  Serial.println(buffer);
}


void loop() {}

The results look good:

from func:   plth/foo/bar
from setup:   plth/foo/bar