1

How is this working? In writing this function, I forgot to include a return statement:

char *getV(const char *var) {
    char *val = nullptr;
    forward_list<formField>::iterator idx;

    formField ffld;
    idx = _fields.begin();
    while (idx != _fields.end()) {
        ffld = *idx;
        if (strcmp(ffld.Name, var) == 0) {
            val = ffld.Val;
            break;
        }
        idx++;
    }
}

Here is formField:

struct formField {
    char    *Name   = nullptr;
    char    *Val    = nullptr;
};

Here's how I'm calling the function:

int main () {
    form fdata;

    fdata.add("FirstName", "Alan");

    char *fval = fdata.getV("FirstName");
    if (fval != nullptr) cout << fval;
    else cout << "not found";
    cout << "\n";
}

I didn't notice the missing return until later, after testing the function and writing other code which uses it. The function is supposed to return *val...and it does! How?

alanlittle
  • 460
  • 2
  • 12
  • 3
    The neverending magic of undefined behaviour. – Quentin May 08 '17 at 21:05
  • Undefined Behaviour. *anything* is allowed by the compiler; what you expect, nothing, a crash, formatting your harddrive. Your code is ill formed, it has no meaning. And no, the compiler is *not* required to give you an error or warning when you engage in UB. It is *your* responsibility to "just not do that". – Jesper Juhl May 08 '17 at 21:05
  • 2
    ...washing the dishes, placing obscene phone calls to random numbers in Moscow.... OK, interesting. Thanks. – alanlittle May 08 '17 at 21:08
  • It works as expected because the compiler puts `val` variable in `RAX` register (assuming standard calling convention in X64). It's just a coincidence. With different compiler it may break horribly. – freakish May 08 '17 at 21:08
  • 1
    @JesperJuhl I understand. It's not something I did intentionally, or intended to leave as-is (unless it turned out to involved some defined behavior I was unaware of). – alanlittle May 08 '17 at 21:14
  • @alanlittle Also note that if you compile with full optimizations then it is possible for you to see different effect because the compiler may fully optimize your function to do nothing, because without the return statement it actually does nothing. You would see the runtime error and/or incorrect behaviour then. It is always a good idea to compile with different flags from time to time. – freakish May 08 '17 at 21:21
  • doing operations with C-Strings in C++ like in C is not recommended. – The Techel May 08 '17 at 21:23
  • @TheTechel Could you point me to something on that? My understanding is that it's a decision between convenience and efficiency, with strings being more convenient (and less touchy) and c-strings being more efficient. Thanks. – alanlittle May 08 '17 at 21:32
  • you would just use std::string or references to one. As far as I see, the values of a 'formField' have to be manually allocated (which is bad) if they are not given statically. You could even replace the entire thing with a std::(unordered_)map – The Techel May 08 '17 at 21:37
  • 1
    @TheTechel That's a useful construct. Thanks. – alanlittle May 08 '17 at 21:45

2 Answers2

6

The calling convention of your platform specifies how the function returns the value it wants to be returned to the caller (e.g: saved in a register). Since the register exists, the caller code just takes the value it finds there, possibly garbage, or the actual good value by chance. Either way, your program invokes undefined behaviour, it cannot be trusted any more.

The solution is to turn the compiler warnings on. E.g, on GCC or Clang:

g++ -Wall -Wextra -Werror
erenon
  • 18,838
  • 2
  • 61
  • 93
3

According to [stmt.return] (emphasis mine):

Flowing off the end of a constructor, a destructor, or a function with a cv void return type is equivalent to a return with no operand. Otherwise, flowing off the end of a function other than main results in undefined behavior.

Usually you can catch these kinds of errors by turning on and reading warnings generated by the compiler (e.g. by using -Wall -Wextra with GCC or Clang).

jotik
  • 17,044
  • 13
  • 58
  • 123
  • This doesn't answer the question. – Lightness Races in Orbit May 08 '17 at 21:11
  • 2
    @BoundaryImposition Yes it does. Undefined behavior can include "returns a value". – jotik May 08 '17 at 21:12
  • And the OP is asking for a description of how that specific outcome has been achieved in this instance. Personally I usually leave it at the advice "don't ask these questions", but simply repeating the premise of the question isn't answering it. – Lightness Races in Orbit May 08 '17 at 21:13
  • Thanks to both of you. "Undefined behavior" is answer enough, and jotik posted first, though erenon's answer is more technically complete. I wish I'd seen the previous questions, but they did not show up in my search. – alanlittle May 08 '17 at 21:21
  • @Dayan I suppose not. If it matters to erenon, I'll change it and accept his/her answer. – alanlittle May 08 '17 at 21:25
  • @alanlittle You should accept the answer you find most useful. Your vote pins it to the top, so make it count. – Baum mit Augen May 08 '17 at 21:27
  • @BaummitAugen They're both informative. While I'm new to C, I understand the idea of `undefined behavior`, so that was sufficient to let me know it's not something I can rely on. I'm just trying to be fair (I'm also new to this forum!) – alanlittle May 08 '17 at 21:30
  • @alanlittle *"I'm just trying to be fair"* The main purpose of the accept vote is not to reward the answer's author, although it does that too. It exists because we trust the OP to be able to evaluate the usefulness of the answers quite well, as OP must use them to solve their problem. Hence the big impact on the default answer sorting. So again: Pick the one you personally found most useful. (Not arguing about your choice here, just explaining the general principle.) – Baum mit Augen May 08 '17 at 21:36
  • @BaummitAugen Good point. Sorry, jotik! – alanlittle May 08 '17 at 21:42
  • @alanlittle No problem! Welcome to Stack Overflow! :-) – jotik May 09 '17 at 06:36