• (nodebb)

    It IS a WTF, because it doesn't handle FILE_NOT_FOUND!

  • (nodebb)

    TRWTF is writing ==FALSE instead of !

  • Greg (unregistered)

    Makes you wonder why they don't return the error code in the first place instead of storing the error somewhere (very likely in some global variable) and returning a BOOL (which in C-land is just an int).

  • Michael R (unregistered)

    Is the long-time developer's name Paula?

  • new arrival (unregistered) in reply to Michael R

    good one

  • Rob (unregistered) in reply to Greg

    Makes you wonder why they don't return the error code in the first place instead of storing the error somewhere (very likely in some global variable) and returning a BOOL (which in C-land is just an int).

    The GetLastError() is a giveaway that this is most likely a Windows API call, which uses the "return false and check GetLastError() afterwards" convention a lot. Fortunately GetLastError() it's thread-safe.

    Note that Unix / Linux also follow this convention a lot. There's the (thread) global errno variable that serves the same purpose as GetLastError().

  • some guy, also expert (unregistered)

    They don't change the code, the code changes them.

    Gaze not into the abyss, lest you become recognized as an abyss domain expert, and they expect you keep gazing into the damn thing.

    -- nickm_tor, apparently

  • f222 (unregistered) in reply to Greg

    For me in C it feels quite common to do it like that.

    In Unix/Linux development there is errno, on Windows we got GetLasterror.

    I'm not saying it's the right thing to do but it's definitely a common idiom: If you don't need details about the error, your code will have nothing to do with the details of the error...

  • (nodebb)

    Unless I'm misreading it because it's still early in the morning (a.k.a. before noon), this is definitely a WTF. If there isn't an error, it handles the error. Or the value of error is the opposite of what the variable name means.

  • (nodebb)

    They handle an error if the error flag is FALSE?!

  • (nodebb)

    This feels even more screwy because if you started with "functions return an int, 0 is success, anything else a failure denoted by the value (or you need to look in errno)", and factor in that int 0 is falsey, and any other value truthy, then surely the error case would be "error == TRUE" ?

  • (author) in reply to Dragnslcr

    Well, they call the variable error, but it's really result or honestly success. It's a badly named variable, not a full on backwards logic.

  • (nodebb) in reply to Remy Porter

    Then the "annoying" convention and the "standard" convention are the same, except that the annoying convention names the variable the exact opposite of what the value means?

    Sure, we've seen worse, and we've seen this one many times, but it's definitely a WTF.

  • Scragar (unregistered) in reply to Rob

    That comes from the very early days. Returning an int pushes 4 bytes onto the stack, and most of the time you only care about success. Those four bytes in every function call on the stack add up quickly, and if you only have 16kb of memory total it's vitally important you save as much memory as possible. To that end, the error number is a global, and we use a single byte that's either 0 or 1 to indicate if you need to check it. Now every function call that needs to care about errors only adds 1 byte to the stack, not 4.

    Now however there's no reason it needs to be used, we can afford to lose 3 extra bytes for each function call that might return an error, but we don't because the standard for how it works has been written and changing things risks breaking things.

  • Anonymous Coward (unregistered)

    I absolutely would do this. Note that BOOL is not the built in Boolean type. I would not treat it as such.

  • Brian (unregistered)

    Having been a C dev a long time ago I can explain some of the history behind this. C doesn't have a built-in bool type, so it's often #define'd as FALSE = 0 and TRUE = 1 (including in the Windows headers). But in C, anything nonzero is considered true, so it's more correct to say TRUE = !FALSE.

    That led to a general rule of never explicitly checking for TRUE. Many overly-zealous devs took that and ran with it, mandating that any "bool" check tested FALSE instead. So it's no big surprise that a long-time developer in a long-running C project would be of that school.

  • Hmmmm (unregistered)

    "OK, function, do your thing and give me the error (eye roll). Oh, no error (eye roll)? Let me go ahead and check the error anyway (eye roll)!" Now all the code needs is the correct contents of the function so that it can do it right itself in the (non-)error-handling section.

  • (nodebb) in reply to Greg

    returning a BOOL (which in C-land is just an int).

    Much better to include <stdbool.h> and use a bool, which is not just an int, and which has "proper" boolean-type semantics, notably squashing all "true" values to true, like in C++.

  • Kelly Hrdina (unregistered)

    Yes, that would annoy me too.

  • Officer Johnny Holzkopf (unregistered)

    When success is error, and error is success, then everything is possible!

  • jgh (unregistered)

    No, the standard is:

    int error = someFunc(); // error=0 is no error if (error) {// handle whichever error occured

    There is only exactly one possible "success", there are an infinite possible "failures", so you map your returns that way, the one to one, and the many to many.

  • (nodebb) in reply to jgh

    There is only exactly one possible "success"

    S_FALSE is knocking on your door, asking you what you mean by "exactly one" success.

    For the benightedly ignorant (or just unaware, whatevs) among the audience, S_FALSE is a "successful" return from calls to COM APIs in Windows, indicating success with a nuance. Its value, unlike S_OK's zero, is one.

    Yes, you heard me right, S_FALSE == 1.

  • (nodebb)

    You can return 0 on error, Then you call it

    BOOL success = some_func(); if (success == false) { // Handle error }

    That's the same semantics as the code, except the misleading variable is renamed.

  • Daemon (unregistered)

    Ah, yes. The classic case of 'Yes, there is no error'. I had a few discussions with co-workers about that one.

    Thankfully I've always managed to sway them.

  • 516052 (unregistered)

    Honestly, this looks like someone came into the project fresh off doing assembly and decided to reimplement the error flag. They just did it kind of wrongly. But I definitively have seen stuff like this back in the day when people were still getting used to C. And it didn't go away either.

  • 516052 (unregistered)

    Ok, I had to brush up a bit on assembly since it's been decades. And TLDR I got things backwards. I was thinking off the flags register. I think. Apparently my memories of what I did in the 90's aren't as sharp as they used to be.

  • Officer Johnny Holzkopf (unregistered) in reply to 516052

    In ye olden tymes upon thy IBM maynframeth, return codes (RCs) allowed a "strange" way of identifying several degress of success: While RC=0 obviously meant "everything okay, operation succeeded as expected", non-zero codes could still mean success, but with limitations or explanation, like RC=2 for "operation completed due to override", RC=4 for "operation completed with minor errors", RC=6 for "operation completed, but not as expected", up to RC=255 for "operation could not be performed, severe error". This of course depended on the actual system and problem programs, as well as how user programs were written. Additional reason codes, like UNIX's errno, could reveal further details that the programmer could query. And obviously, documentation was key - so you had to implement your program flow and JCL accordingly in order to avoid repeating or restarting job steps for something that was "sufficiently successful" for your specific task... yes, I am old... and i'm not sure I remember this entirely correctly...

  • Kotarak (unregistered)

    Somehow I don't get the examples. Shouldn't the functions return 0 on success and non-zero (often -1) for errors? So shouldn't the pattern be the following?

        int error = some_func();
    if (error) {
        error_handling_here();
    }

    Regarding the original submission, I also think that it is a RWTF, because the name of the variable does not relate well to the condition in the if. You test for "error" being false and then do error handling? Wait! what? That is totally confusing. success == false or error == true would make much more sense.

  • 516052 (unregistered) in reply to Officer Johnny Holzkopf

    You don't have to reach that far back. I've seen stuff like that as recently as today. Just look at the Metatrader API. And yes, that Metatrader. The company that runs the largest Forex retail platforms. Their entire scripting language, and from what I've seen their server plugin API as well runs on the pattern of returning an integer result code that you than match against an enum and if the numbers match one of values that mean error you query GetLastError() to see what actually happened.

  • Quirkafleeg (unregistered)

    The fact that "other developers follow" says as much about them as it does the "long term developer"...

  • 516052 (unregistered) in reply to Quirkafleeg

    Rule one of working on any project is that you always follow the patterns and conventions laid out by said project.

    The only thing that sucks more than having a bad set of those (which may or may not have been good ages ago) is having a multiple of competing standards where each part of the project does things its own way.

    If something is done wrong, but consistently wrong you can at least learn and understand "the way". If different parts of the codebase all do things their own way than you can't even do that.

    Which is why there is a special place in hell reserved for people that break with conventions.

  • Craig (unregistered) in reply to Steve_The_Cynic

    There are clear conventions with COM coding, at least, and things are very readable as long as you follow them. If you don't care about a specific HRESULT like S_FALSE then just use the SUCCEEDED macro, and then you don't need to worry about the details of how the bits are assigned in a proper HRESULT.

    Possibly TRWTF is that I like the COM result conventions enough that I've used them in non-COM code.

Leave a comment on “The Error Check”

Log In or post as a guest

Replying to comment #701969:

Log Out

« Return to Article