Returning reference

I have been trawling the net and found something that conflicted my understanding and wondered if you could tell me if you think this is correct?

double & GetWeeklyHours()
{
    double h = 46.50;

    double &hours = h;

    return hours;
}

Surely returning h, which is created IN the function, will be freed from memory the moment the function ends?

You are correct. But that will probably work because what really happens is a value copy.
If you were to save the pointer to that float (and mess with the stack [say, creating a new variable]) it will be corrupt when you try to use it.

I think this will work until the next function call because the value will sit on the stack. So

x = GetWeeklyHours();

should be OK but don't expect it to be there half an hour later :slight_smile:


Rob

What happens if you write "GetWeeklyHours () = 30.0;" ?
:slight_smile:

AWOL:
What happens if you write "GetWeeklyHours () = 30.0;" ?

Probably nothing bad, since you are writing into an, at that point, unused part of the stack.

The big issue is that this (unused) part of the stack could be smashed by any other function call or an ISR. It's almost guaranteed to be overwritten within 1,000 microseconds by the Timer 0 ISR, for example.

Good point about ISRs gardner, I'll change my code to

noInterrupts();
x = GetWeeklyHours();
interrupts();


Rob

Graynomad:
noInterrupts();
x = GetWeeklyHours();
interrupts();

It's simply that cool, to return things by reference. Especially when it's done for no apparent reason at all.
8)

Beakie:
C++ Examples: Returning a Reference

Heh, yeah that's funny. It's actually given as an example of how to do things. I think it does work, as a previous poster mentioned, because it's a 'value type'. Ergo, the compiler may just be returning the whole value and discarding the reference. Or alternately just out of luck with the stack as the others said.

Beakie:
I...wondered if you could tell me if you think this is correct?

It's crap. Period. Full stop.

Use of the returned value will result in undefined behavior.

From the C++ language standard ISO/IEC 14882

In section 3.7, line 4


The lifetime of a reference is its storage duration


In section 3.7.2, line 1


Local objects ... not explicitly declared static or extern have automatic storage duration. The storage of these objects lasts until the block in which they are created exits.


In section 1.3.2 the definition of undefined behavior is given as


behavior, such as might arise upon use of an erroneous program construct or erroneous data, for which this International Standard imposes no requirements.


Here's the thing: Use of the returned value of the example function may give the value that the programmer expected. It may give junk. Showing that it "works" in a specific program for a particular function compiled by a particular version of a particular compiler doesn't make it OK. Undefined is undefined.

Note that if you test something and it seems to "work," that doesn't mean that the code is correct. Because of the ever-present possibility of undefined behavior, you can not, in general, prove a program is correct by testing alone. I mean, you should test, test, and test some more, but you still gotta write correct code.

Bottom line: Reuse of storage beyond its lifetime is undefined behavior. Compilers are not required to give warning or error messages for this specific type of undefined behavior.

Regards,

Dave

Footnotes:

Graynomad:
Good point about ISRs gardner, I'll change my code to

noInterrupts();

x = GetWeeklyHours();
interrpts();

I respectfully recommend that you make maximum attempts to write code that does not ever result in undefined behavior. Disabling interrupts does not make the behavior defined.

maniacbug:
I think it does work,,,, because it's a 'value type'

It is undefined behavior. It is not a "value type." It is a reference. If a function returns a reference to a local variable, the result is worthless.

AlphaBeta:
But that will probably work because what really happens is a value copy.

No, there is no copy involved. The function returns a reference, not a copy of anything. If it seems to "work" by displaying the wrongfully-expected value, well, that's too bad, since the programmer might assume that the code is correct. The code is crap. (But I said that already.)

Yes I agree with all the above. I was just trying to show that it probably would work and didn't mean to imply that it is good practice.

Certainly I have never returned a reference like that and can't see any reason to do so.


Rob

Graynomad:

noInterrupts();

x = GetWeeklyHours();
interrupts();

I'm not sure that's good enough. You don't mention the type of 'x', and I am not really all that clear on exactly how C++ references differ from C pointers at this level of detail. If 'x' is still a reference, it is to an undefined thing. You need to dereference 'x' too, with interrupts blocked.

I can recall having to do exactly this sort of thing in real life with badly coded vendor libraries. It's amazing the experiences you used to get back when things were coded in C and C++. Nowadays its all friggin Java, and if you want to know what the idiots who coded it did, you just decompile it and look. Back in the day I recall snooping around with DBX and suddenly realizing this pointer the X.25 library was returning was pointing into the stack -- no wonder the message was passing basic validation and then later appearing to change to an invalid one! Bitching at the vendor would take weeks, and adding some sigblock() and sigsetmask() code and copying the data to a safe place took 20 mins. Way worse was the memory leak that resulted from them allocating something and returning a pointer into the middle of the block. It took a good deal of debugging to determine the correct offset to the beginning of the block so we could free it. Again, the fix would take weeks, but the hack could be done in minutes.