Return by coincidence

tdb

This is a rather minor WTF, but maybe you'll get a chuckle out of it. I just discovered it in one of my hobby projects:

GlDecoder *glprint_new(char *buffer, unsigned bufsize)
{
	GlDecoder *dec;
	GlPrintData *gpd;
gpd = (GlPrintData *)malloc(sizeof(GlPrintData));
gpd->buffer = buffer;
gpd->bufsize = bufsize;
if(!gpd->buffer)
{
	if(!gpd->bufsize)
		gpd->bufsize = 1024;
	gpd->buffer = (char *)malloc(gpd->bufsize);
	dec = gldecoder_new(gpd, glprint_data_free);
}
else
	dec = gldecoder_new(gpd, free);

init_print(dec);
dec->gldError = print_gldError;

}

Take a look at the end of the function and observe the lack of a return statement. Yet this function has been in the project since its beginning, had the last line added to it later, and managed to work correctly all this time. How is that possible? Spoiler below, highlight to reveal.

Take a look at the disassembly dump:

        init_print(dec);
      86:       8b 45 fc                mov    eax,DWORD PTR [ebp-0x4]
      89:       89 04 24                mov    DWORD PTR [esp],eax
      8c:       e8 fc b4 02 00          call   2b58d <init_print>
        dec->gldError = print_gldError;
      91:       8b 45 fc                mov    eax,DWORD PTR [ebp-0x4]
      94:       c7 80 b4 1f 00 00 d2    mov    DWORD PTR [eax+0x1fb4],0xd2
      9b:       00 00 00

Each of the last two lines loads the dec pointer into eax and does stuff with it. Coincidentally, that's exactly the value I wanted to return from the function.

Time to turn on -Wall I guess (forgetting that up until now is another minor WTF).

SCB

Casting the return value from malloc?

WTF?

Dr_Frankenstein

Yay, when EAX garbage turns out to be the right thing... which happens quite often actually.

By the standard, it's considered as undefined behaviour though, IIRC.

@SCB said:

Casting the return value from malloc?

WTF?

Not required in C, but required in C++. Also sometimes used in C for "correctness"

See http://c-faq.com/malloc/cast.html .

PJH

@Dr Frankenstein said:

Not required in C, but required in C++.

You should be using new, not malloc() in (new) C++ code.

Also sometimes used in C for "correctness"

There is nothing correct about using malloc() in a C program and not including <stdlib.h> for which this cast is the 'fix.'

tdb

@PJH said:

Also sometimes used in C for "correctness"
There is nothing correct about using malloc() in a C program and not including <stdlib.h> for which this cast is the 'fix.'

The cast makes the compiler complain if I try to assign it to a pointer of wrong type by accident. Consider this code:

short *array;
array = malloc(n_elements*sizeof(short));

C allows implicit conversion from void * to any pointer type, so all is well and good. Now, if I later decide that I need to increase the size of the data type, it might result in this:

int *array;
array = malloc(n_elements*sizeof(short));

Whoops, we now actually only have memory for n_elements/2 ints (assuming the common case of sizeof(int)=2*sizeof(short)). Had I done this instead, I would have gotten an error from the compiler abotu incompatible pointers:

int *array;
array = (short *)malloc(n_elements*sizeof(short));  /* Error, can't assign short * to int * */

So the typecast is a kind of safeguard. Granted, it's possible to only change the typecast and miss the sizeof, but at least they're on the same line, while the declaration may be much further away (possibly inside a struct declaration in a different file).

PJH

@tdb said:

The cast makes the compiler complain if I try to assign it to a pointer of wrong type by accident. Consider this code:
short *array;
array = malloc(n_elements*sizeof(short));
[...] 

Wrong way of writing it. What happens when you change the type of array? You now have to change your code (unnecessarily) in at least two more places.

What you should be doing is writing your code so it doesn't matter what type it is:

short *array;

[lots of lines]...

array = malloc(n_elements*sizeof(*array));

@tdb said:

Granted, it's possible to only change the typecast and miss the sizeof, but at least they're on the same line

This is why I [code]#define ALLOC(x) ( (x *) malloc( sizeof(x) ) )[/code]

Edit: but PJH has got a point, about [code]sizeof(*array)[/code]

Daid

Wait.... there are people that think implicit typecasts are a good thing?

DescentJS

@Daid said:

Wait.... there are people that think implicit typecasts are a good thing?

Depends on what the types are. short or char --> int or __int64 should be just fine as an implicit conversion.

Daid

@DescentJS said:

@Daid said:
Wait.... there are people that think implicit typecasts are a good thing?

Depends on what the types are. short or char --> int or __int64 should be just fine as an implicit conversion.

char c = 0x80;
printf("0x%x\n", (c & 0x0100));

Expected result? Real result? With GCC 4.3.3 you get 0x100, not sure if that's the same for all compilers. But I think quite a few people would expect 0x0

Ilya_Ehrenburg

@Daid said:

@DescentJS said:
@Daid said:
Wait.... there are people that think implicit typecasts are a good thing?

Depends on what the types are. short or char --> int or __int64 should be just fine as an implicit conversion.
char c = 0x80;
printf("0x%x\n", (c & 0x0100));

Expected result?

Usually, 0x100 because char is signed, so 0x80 = -128, (-128)&256 == 256.

Real result? With GCC 4.3.3 you get 0x100, not sure if that's the same for all compilers. But I think quite a few people would expect 0x0

If you have a machine which has unsigned char as a standard, it's going to be 0. To be sure to get the expected results, use signed char/unsigned char resp. -f(un)signed-char.

If you're using decimal notation, there's no surprise at all, if you're using hexadecimal notation, you should be aware that the widening conversions use sign-extension.

shadowman

Yeah, I guess it makes sense that it worked out. On an x86 system the function return value would likely be stored in eax. Missing that, eax would have the last expression results. Which it turned out was the intended return anyway. Of course, this would be a horrible thing to do on purpose, given it's system dependent and relies on UB as far as your c++ compiler is concerned.

Makes me wonder why something like that doesn't generate a compiler error by design.

SenTree

@shadowman said:

Makes me wonder why something like that doesn't generate a compiler error by design.

It generates a warning. As tdb said, he had warnings turned off:

@tdb said:

Time to turn on -Wall I guess (forgetting that up until now is another minor WTF).

Ilya_Ehrenburg

@SenTree said:

It generates a warning. As tdb said, he had warnings turned off:

Well, yes, that's better than nothing. But not returning a value from a non-void function is a serious issue. IMO, it shouldn't compile. Even though C compilers assume you know what you're doing, if you say you will return a value and then don't, you clearly (momentarily) don't know what you're doing, so the compiler should force you to reconsider.

derula

@Ilya Ehrenburg said:

Well, yes, that's better than nothing. But not returning a value from a non-void function is a serious issue. IMO, it shouldn't compile. Even though C compilers assume you know what you're doing, if you say you will return a value and then don't, you clearly (momentarily) don't know what you're doing, so the compiler should force you to reconsider.

Compilers are built that way that they error if and only if it is decidable whether or not the code contains an error. In this case, it is obviously not decidable, since the code lacking the return worked without any problems. Thus, only a warning is emitted. Taught in the first or second lecture of a Compiler construction course.

Well, I say wherever the compiler decides to give out a warning, give out an error instead. 'cause it's darned obvious the programmer was distracted.

If I wrote Ruby, I'd use the [code]return[/code] keyword every time.

derula

@Zecc said:

Well, I say wherever the compiler decides to give out a warning, give out an error instead. 'cause it's darned obvious the programmer was distracted.

Ahem? Decidability!

But yeah, the option to disable warnings should be illegal.

@Zecc said:

If I wrote Ruby, I'd use the <font face="Lucida Console" size="2">return</font> keyword every time.

But leaving it out is so much more fun! Just like leaving out the brackets around function arguments.

tdb

@derula said:

But yeah, the option to disable warnings should be illegal.

Also, it should give more warnings by default. I didn't have to use any option to disable the warning about a missing return statement. The compiler is GCC 4.3.2, in case anyone was wondering.

SlyEcho

This reminds me of Groovy where you can omit the return statement:

int add(int a, int b) {
    a + b
}

Or C# Lambdas where the lambda body is a single expression that is returned by default.

Func<int, int, int> add = (a, b) => a + b;

This can be very powerful when combined with method calls that return this (StringBuilder.Append):

Func<IEnumerable<string>, string> join = strings =>
    strings.Aggregate(new StringBuilder(), (sb, s) =>
        (sb.Length > 0 ? sb.Append(", ") : sb).Append(s)).ToString();

@derula said:

Ahem? Decidability!

Like I said: when the compiler decides to give out the warning. =P

SenTree

@tdb said:

@derula said:
But yeah, the option to disable warnings should be illegal.
Also, it should give more warnings by default. I didn't have to use any option to disable
the warning about a missing return statement. The compiler is GCC 4.3.2, in case anyone was wondering.

Interesting. Is the warning disabled by default, or non-existent ? I think all of the (embedded C) compilers I've used give that warning by default. I should think lint isn't too happy with it either.

amischiefr

@SenTree said:

Interesting. Is the warning disabled by default, or non-existent ? I think all of the (embedded C) compilers I've used give that warning by default. I should think lint isn't too happy with it either.

Good question. My teachers always made us compile with -Wall, and if we had warnings that was points off. I never took the time to learn which warnings always show up and which ones show up with -Wall or -pedantic.

SenTree

@amischiefr said:

@SenTree said:
Interesting. Is the warning disabled by default, or non-existent ? I think all of the (embedded C) compilers I've used give that warning by default. I should think lint isn't too happy with it either.

Good question. My teachers always made us compile with -Wall, and if we had warnings that was points off. I never took the time to learn which warnings always show up and which ones show up with -Wall or -pedantic.

Wise teachers! Most of my work is safety-related, so I always use -Wall or equivalent and aim for zero warnings, even during development. That way, I don't have to go back and clean up at the end of the project when there's never any time.

SenTree

@amischiefr said:

@SenTree said:
Interesting. Is the warning disabled by default, or non-existent ? I think all of the (embedded C) compilers I've used give that warning by default. I should think lint isn't too happy with it either.

Good question. My teachers always made us compile with -Wall, and if we had warnings that was points off. I never took the time to learn which warnings always show up and which ones show up with -Wall or -pedantic.

Wise teachers! Most of my work is safety-related, so I always use -Wall or equivalent and aim for zero warnings, even during development. That way, I don't have to go back and clean up at the end of the project when there's never any time.

Ilya_Ehrenburg

@derula said:

@Ilya Ehrenburg said:
Well, yes, that's better than nothing. But not returning a value from a non-void function is a serious issue. IMO, it shouldn't compile. Even though C compilers assume you know what you're doing, if you say you will return a value and then don't, you clearly (momentarily) don't know what you're doing, so the compiler should force you to reconsider.

Compilers are built that way that they error if and only if it is decidable whether or not the code contains an error. In this case, it is obviously not decidable, since the code lacking the return worked without any problems. Thus, only a warning is emitted. Taught in the first or second lecture of a Compiler construction course.

Okay, I'm apparently dense today. I would've thought that it's not only decidable, but in fact almost trivial to check whether a function declared non-void contains a return-statement (as last instruction, at that).

javac does.

SenTree

@Ilya Ehrenburg said:

@derula said:
@Ilya Ehrenburg said:
Well, yes, that's better than nothing. But not returning a value from a non-void function is a serious issue. IMO, it shouldn't compile. Even though C compilers assume you know what you're doing, if you say you will return a value and then don't, you clearly (momentarily) don't know what you're doing, so the compiler should force you to reconsider.

Compilers are built that way that they error if and only if it is decidable whether or not the code contains an error. In this case, it is obviously not decidable, since the code lacking the return worked without any problems. Thus, only a warning is emitted. Taught in the first or second lecture of a Compiler construction course.

Okay, I'm apparently dense today. I would've thought that it's not only decidable, but in fact almost trivial to check whether a function declared non-void contains a return-statement (as last instruction, at that).
javac does.

Not dense. The check is trivial; however C compilers treat it as a warning, not an error, on the assumption that you know what you're doing. Whether it should be a warning (non-fatal) or an error (fatal) is another question. tdb's original WTF was that he had the warning turned off - illustrating that treating it as a warning is not such a good idea.

Derula's digression into decidability is a bit of a red herring, and possibly confuses language syntax errors with application logic errors. [disclaimer: I may have misunderstood his meaning; I am not trying to start a fight!].

tdb

@amischiefr said:

@SenTree said:
Interesting. Is the warning disabled by default, or non-existent ? I think all of the (embedded C) compilers I've used give that warning by default. I should think lint isn't too happy with it either.

Good question. My teachers always made us compile with -Wall, and if we had warnings that was points off. I never took the time to learn which warnings always show up and which ones show up with -Wall or -pedantic.

The exact warning option in question is -Wreturn-type, and it's included in -Wall. My usual set of warning options is -Wall -Wextra -Wshadow -Wpointer-arith -Werror; however, this project required some features my normal build system isn't capable of (yet), so I had to write a makefile by hand and forgot to put those options in. This will rectified in the immediate future.

lolwtf

@Zecc said:

Well, I say wherever the compiler decides to give out a warning, give out an error instead. 'cause it's darned obvious the programmer was distracted.
If I wrote Ruby, I'd use the <font face="Lucida Console" size="2">return</font> keyword every time.

I believe that's -fatal-warnings. Might be another letter out front.

moz1

@SenTree said:

Not dense. The check is trivial; however C compilers treat it as a warning, not an error, on the assumption that you know what you're doing. Whether it should be a warning (non-fatal) or an error (fatal) is another question. tdb's original WTF was that he had the warning turned off - illustrating that treating it as a warning is not such a good idea.

It isn't as easy to subclass things in C as it is in Java. Because of this, you may sometimes see a function which has parameters it never uses or a return type its caller will never check simply because it is designed to have the same type as another function which actually needs these things.

For instance, you may see a function declared as int main(int, char**) which is only ever left via a call to exit() or an external event.

shadowman

@derula said:

@Ilya Ehrenburg said:
Well, yes, that's better than nothing. But not returning a value from a non-void function is a serious issue. IMO, it shouldn't compile. Even though C compilers assume you know what you're doing, if you say you will return a value and then don't, you clearly (momentarily) don't know what you're doing, so the compiler should force you to reconsider.

Compilers are built that way that they error if and only if it is decidable whether or not the code contains an error. In this case, it is obviously not decidable, since the code lacking the return worked without any problems. Thus, only a warning is emitted. Taught in the first or second lecture of a Compiler construction course.

I suppose there could be a rule that a non-void function must contain a return statement -- that would certainly be decidable.

@c++ standard said:

Flowing oﬀ the end of a function is equivalent to a return with no value; this results in undeﬁned behavior in a value-returning function.

So I guess the language designers, for whatever reason, decided to leave this up to the compiler writers. It's technically not malformed syntax, just undefined.

morbiuswilters

@shadowman said:

@c++ standard said:
Flowing oﬀ the end of a function is equivalent to a return with no value; this results in undeﬁned behavior in a value-returning function.
So I guess the language designers, for whatever reason, decided to leave this up to the compiler writers. It's technically not malformed syntax, just undefined.

So either Stroustrup is a crackhead or a comedic genius who wanted to gift us with WTF material for decades.

PJH

@shadowman said:

@c++ standard said:
Flowing oﬀ the end of a function is equivalent to a return with no value; this results in undeﬁned behavior in a value-returning function.
So I guess the language designers, for whatever reason, decided to leave this up to the compiler writers. It's technically not malformed syntax, just undefined.

Undefined meaning (in this context) that it's perfectly valid for the compiler to produce a program that will format your hard drive. See also: Nasal Demons.

No - this is more 'leaving it up to the software writers,' as in "you should always return something from these sorts of functions; you cannot rely on the behaviour if you don't."

Of course the better compilers may offer a warning in these situations, but they don't have to.

derula

@SenTree said:

Derula's digression into decidability is a bit of a red herring, and possibly confuses language syntax errors with application logic errors. [disclaimer: I may have misunderstood his meaning; I am not trying to start a fight!].

Okay maybe I was too fast, but in fact, there is no application logic error. Everything works as it should, and it doesn't seem to be in the standard that a non-void function needs a return. If it was, it no doubt would need to emit an error. I'm not familiar with the C++ standard, that's just what I thought: it's not in the standard, therefore leaving it out is not an error because the code might still work correctly.

But nah, don't want to start a fight either, probably I'm just plain wrong.

tdb

@derula said:

@SenTree said:
Derula's digression into decidability is a bit of a red herring, and possibly confuses language syntax errors with application logic errors. [disclaimer: I may have misunderstood his meaning; I am not trying to start a fight!].

Okay maybe I was too fast, but in fact, there is no application logic error. Everything works as it should, and it doesn't seem to be in the standard that a non-void function needs a return. If it was, it no doubt would need to emit an error. I'm not familiar with the C++ standard, that's just what I thought: it's not in the standard, therefore leaving it out is not an error because the code might still work correctly.

But nah, don't want to start a fight either, probably I'm just plain wrong.

The C++ standard defines undefined behaviour thusly:

@C++ 2003 said:

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

I take this to mean that the program or data is basically invalid, but the standard requires no diagnostic. I seem to recall that the term "ill-formed program" (meaning that the program is invalid, period) is used in the context of syntax errors, while things like dereferencing a NULL pointer are undefined behaviour. Then there's also the term "implementation-defined" which means that the program is well-formed and must be accepted by the compiler, but it's still free to interpret the code in whichever way is best suited for the platform. Such as running nethack when the program contains a #pragma preprocessor statement.

As for application logic error... I'm not sure if this can be called a logic error in the strictest sense, but it's damn near one. It might happen to work with this particular compiler, and with these particular options; however, even the simple act of turning optimizations on has a high chance of breaking it.

moz1

@shadowman said:

@c++ standard said:
Flowing o\ufb00 the end of a function is equivalent to a return with no value; this results in unde\ufb01ned behavior in a value-returning function.

So I guess the language designers, for whatever reason, decided to leave this up to the compiler writers. It's technically not malformed syntax, just undefined.

That's slightly stricter than the [url=ftp://202.38.72.38/pub/eBooks/ANSI%20C/C%20Standard/ISO%20C%20FDIS%201999-04.pdf]C version[/url], which only states that "If the } that terminates a function is reached, [i]and the value of the function call is used by the caller[/i], the behaviour is undefined" (my italics). It's not possible to tell whether the function in the OP is correct at compile time, so there's no compile-time error. And there's nothing to trigger a special run-time error, as that would just waste bytes in a working program.

Ilya_Ehrenburg

@_moz said:

That's slightly stricter than the C version, which only states that "If the } that terminates a function is reached, and the value of the function call is used by the caller, the behaviour is undefined" (my italics). It's not possible to tell whether the function in the OP is correct at compile time,

Yes, in general you cannot tell whether the function's return value is ever used at compile time. But if a function is declared with a return type, there's a good chance that somebody sometime will use the return value. Therefore, IMO, the standard should mandate a "return value; " at the end of a non-void function.