• LCrawford (unregistered)

    Frist of all, this code is pretty decent, compared to traditional C code - at least the file is validated, which is more than most code from that era does. The fact that it doesn't diagnose which structure isn't important, especially for end-user facing code. If it were an audio file editor, additional information might be useful. Otherwise it's similar to the throw new AppException(BAD_PARAMETER), not telling which parameter or what the problem was with the parameter.

  • Mordac The Preventer (unregistered)

    You claim that this is C++, but it looks like C code to me. Maybe the codebase was migrated and they never needed to refactor this, or maybe it originated in copypasta of some sort.

    TBH I've used the "do{ ... } while(false)" idiom in a lot of code over the years. The only WTF is "(time(0L) == 0)" ...

  • Vilx- (unregistered)

    But then there's also the strncmp() function which allows to specify that you want to compare EXACTLY 4 bytes, no more no less, so the whole mucking around with the zero terminator is simply unnecessary.

  • some guy (unregistered)

    The do-while-false has the advantage over the goto that it can only ever go down. But yes, an exception is here the correct choice, even if the interface demands that the module does not throw, but reports error codes.

  • (nodebb)

    Which just goes to show, TRWTF is C-style strings.

    Frankly, most of the examples on this site show that TRWTF is programmers.

  • (nodebb) in reply to Vilx-

    there's also the strncmp() function which allows to specify that you want to compare EXACTLY 4 bytes, no more no less

    Read. The. Docs. More. Carefully.

    strncmp compares "at most N characters", not "exactly N bytes". If either string is shorter than N, or if the strings are in some other way different in the first N characters, the function will stop early. Operating on characters and operating on bytes are usually (almost) equivalent, by they aren't identical.

    As I said above, TRWTF is programmers.

  • {Error:GET_NAME_FAILED} (unregistered)

    I would just have moved the content of that do-while loop to a function and broken out of it with return in case of an error.

  • RLB (unregistered) in reply to Steve_The_Cynic

    TRTRWF is Python-and-Java-level programmers bitching about C.

  • (nodebb)

    Frankly, most of the examples on this site show that TRWTF is programmers.

    I replaced a light fixture over the weekend. After I finished, I flipped the breaker back on and tested the light. I was 99.9% confident that it would work exactly as I expected it would and I ended up not being surprised. Then, I thought to myself "If I had spent the past hour writing software, it almost certainly would have not quite worked properly and needed a few more tweak/test cycles to get right". TRWTF is our entire industry.

  • Spike (unregistered)

    The author seems to think structured exception handling would be the best solution. But why not just use standard C++ exceptions?

  • (author) in reply to Spike

    C++ exceptions are an implementation of structured exception handling. "Structured exception handling" is just "there are built-in language mechanism for tossing errors around, instead of stuff like returning error codes from functions."

  • Sole Purpose Of Visit (unregistered)

    Well, it's C. And it's an entirely internalised state-driven parsing algorithm. What on earth would be the point of using exceptions, "structured" or not?

    I'd set the state machine out with more clarity (a simple switch statement would do). I'd use memcmp instead of str(n|ni)cmp. But I wouldn't be writing a simple string parser in C as if I were using Java/Python/C#/a sledgehammer to crack a nut.

    This isn't a WTF. It's just bog-standard incompetent coding.

  • Kerry (unregistered)

    Yeah, this looks like C to me, where i don't remember exceptions. But, it's been 20+ years since I was in that world, maybe i don't remember right.

    This conversation of parsing messages reminds me of when I first started at the current job. There's a process to parse incoming messages which originated from SMS that I needed to tweak.

    Looked at the code, if returned true/false, and had out variables for the different pieces of the message.

    Needless to say, the first thing I did was refactor to return an object representing the parsed message, and throw exceptions for the 'false' cases.

  • (nodebb)

    Sorta reminded me of a microprocessor I once made out of a CPLD. It had about 260 instructions.

    Opcodes 01 to FF were 'output value to DAC and wait 125 uS'. Opcode 00 was a prefix. It had a single index register and a program counter. By preloading the index register you could jump to an address, with return value swapped from PC. You could command the index register to count to zero at once per 125uS for delays. You could conditionally jump based on 4 inputs, and set or clear 4 outputs. It had a 4 bit page address register you could load so it could run code in up to 256k bytes.

    A

    It ran a stupid toy that drove forwards muttering to itself until it hit a wall. It would swear, backup and pretend to dial 911 or 999 .. It wausing a windscreen wiper motor an elastic bands and wooden wheels, s a demonstration of Telematics in about 1995... ll done with 160 macrocells in the CPLD, and a 2 megabit EPROM we nicked from the eraser in the lab..

    Addendum 2020-05-04 08:40: The audio was upsampled 4x before driving the speaker.

    I did it to evaluate a high level design tool we had in our lab for a month or too. So yes, goto had a sound if you got it wrong.

  • Sole Purpose Of Visit (unregistered) in reply to Sole Purpose Of Visit

    (Never been able to find the addendum button on the new site)

    I say "string parser," but it isn't, is it? That's a category error. It's a (ASCII)byte-stream parser. Which is why you use memcmp rather than strcmp. (If the byte stream goes over a page boundary and causes a paging fault, that's not your fault -- it's the fault of the caller.)

    Mithering about how awful "strcmp" might be is a category error. If you're genuinely dealing with proper C strings (or marshalled versions thereof), strcmp does what it says on the tin. If you're not -- just use the appropriate tool.

  • Sole Purpose Of Visit (unregistered) in reply to Remy Porter

    Well, OK, if you say so. I always thought that "Structured Event Handling" was a Microsoft notion (SEH) that applies only to C code written on a Microsoft platform.

    The origins of C++ exception handling are way back there in the mists of time, but me, I'd just call it "exception handling." It does have a few extra "goodies" thrown in, such as the global handlers that allow you to override certain program terminating behaviours, but I'd struggle to find a reason to call it "structured," let alone claim that it's "a form of".

  • Brian (unregistered)

    There's a certain school of thought that says exceptions should be used for exceptional problems, not general control flow. Otherwise it's just a dolled-up goto. Hey, wait a minute...

    If I were writing this, I also would not even think to use exceptions, although I also wouldn't think to use a wrapping loop. I'd either go with returning as soon as a parsing error was encountered, or (for those who also oppose multiple returns) just check the "ok" flag before each segment of parsing.

    However, if this really is C++, which it probably is because C doesn't have a bool type, then one should know that initializing an array as "char id[5] = {0}" would initialize all the elements to 0, so that extra initialization line is unnecessary.

  • Foo AKA Fooo (unregistered) in reply to LCrawford

    You say it's not important, I strongly disagree. Yes, I know end users are stupid and we are the only ones bright enough to even read error messages. But actually, even a stupid end user might want to know if they mistyped the file name, or have no access permission, or there was an actual disk I/O error, the file is of a wrong type or too big to handle, or its contents were corrupted, etc.

    Any form of "general error" is a WTF in my book. There's always a reason for the error if you go deep enough, and hiding that reason from the user is despicable IMNSHO.

  • (nodebb) in reply to Sole Purpose Of Visit
    I'd struggle to find a reason to call it "structured," let alone claim that it's "a form of".
    In C, without Exception Handling, you would usually indicate that your function failed by returning a value that indicated what kind of failure you encountered; 0 indicated it worked. In fact that is how you finished your program as well, passing an integer between 0 and 255 to the operating system. No other information was usually presented. That way you look at the return value and realize "Oh, a block-checksum validation failed somewhere in the file." Now you knew what to search for.

    In C++, with Exception Handling, when you ran into a problem, you would throw a custom Exception instance, which would include all the diagnostic information needed to identify what went wrong, as well as the information needed to find and fix the error, such as Expected Checksum, Read Checksum, Block Number, Location of Error, etc.

    Structured Exception Handling is an extension to C/C++ that Microsoft added to their compilers to allow a version of try-catch-finally exception handling in C. You would __throw an unsigned int when you ran into a problem, to be caught in a following __except(unsigned int) block, depending on the number. After that, the __finally block, if present, would be run. A bit of a bodge that I never used.

    Structured Exception Handling was also implemented in C++ (if it is in C, it has to be in the proper superset C++ too), but even Microsoft recommends using the regular try-catch-finally structure instead of __try-__expect-__finally mechanism of Structured Exception Handling.

    BTW, Event Handling is something else entirely.

  • (nodebb) in reply to Nutster

    Structured Exception Handling is an extension to C/C++ that Microsoft added to their compilers to allow a version of try-catch-finally exception handling in C. You would __throw an unsigned int when you ran into a problem, to be caught in a following __except(unsigned int) block, depending on the number. After that, the __finally block, if present, would be run. A bit of a bodge that I never used.

    No. SEH is a built-in part of all proper Win32-derived Windows OSes (WinNT line, Win9x line, maybe Win3.1-with-Win32s although that hardly counts as a proper Win32-derived OS).

    Structured Exception Handling was also implemented in C++ (if it is in C, it has to be in the proper superset C++ too), but even Microsoft recommends using the regular try-catch-finally structure instead of __try-__expect-__finally mechanism of Structured Exception Handling.

    Prior to Visual C++ 6.0, the two mechanisms were separate, but VC++6 altered the C++ exception system to use SEH behind the scenes. One oddball consequence of that is that a catch (...) will catch segfaults and other OS-level exceptions in a thoroughly undiagnosable (outside a debugger, where you can break on the "first chance" exception) way.

    Oh, and C is not a proper subset of C++. There are a few things left over from the Stone Age, as it were, in C that are not in C++.

  • (nodebb) in reply to Brian

    However, if this really is C++, which it probably is because C doesn't have a bool type, then one should know that initializing an array as "char id[5] = {0}" would initialize all the elements to 0, so that extra initialization line is unnecessary.

    C99 added #include <stdbool.h> which defines bool as an alias of _Bool, and which has essentially the same behaviour as C++'s bool.

  • ooOOooGa (unregistered) in reply to Jaime

    Well yeah, but when you are only dealing with one bit worth of information (three if the fixture has a 3-speed ceiling fan), it isn't hard to get the logic correct the first time.

  • Kleyguerth (github) in reply to Jaime

    To be fair replacing a light fixture is like installing an update, not like writing software. You didn't design the light fixture from scratch, you bought it and installed it. Try designing one from scratch, without copying an existing one, and see if you can do it first try.

  • Perri Nelson (unregistered) in reply to Foo AKA Fooo

    I disagree. Security exceptions are a perfect reason to hide information from the user. There are times when even revealing the existence of a resource to a user that does not have permissions to it is a security flaw.

  • (nodebb) in reply to Kleyguerth

    However, that doesn't change what was going through my head. Any electrician can give his journeyman some work to do, and after doing a basic inspection of the work, he can sleep at night knowing that it's not only going to work, but it's very unlikely to fail in the next fifty years. The fact that I did something simple wasn't at the root of why I knew it was going to work.

    In the software industry, we have a very good grasp of some of our problems (e.g. security). Yet, implementing fixes continues to elude us.

    As an example: the Spectre/Meltdown processor bug fiasco may have happened in 2018, but it was first noted as a security weakness in 1995.

    Try designing one from scratch, without copying an existing one, and see if you can do it first try.

    I can if I follow the guidance spelled out in electrical codes and safety standards. Also, that industry requires not only compliance with standards, but actual testing (in the case of products) or inspection (in the case of installation). In the software industry, we have been unable to achieve reliability and/or security even on systems where that is a stated goal. We struggle with testing and inspection and half of our industry outright rejects the necessity of either.

  • (author) in reply to Sole Purpose Of Visit

    I actually didn't know Microsoft co-opted the term for their own product. "Structured exception handling" is a term with much broader use.

  • sizer99 (google)

    This really looks like pure C to me too. There's the 'bool', but I have tons of old C code with bool typedeffed to an enum containing TRUE, FALSE and FILE_NOT FOUND; and later on there was stdbool.h. bool was too useful not to have in C, so my C code always had bool - one way or another.

    This may have been compiled in C++ but I would guess the coder did in fact not know how to use C++ exceptions, and/or that it was really old C code just copypasted into new code - RIFF file format goes way back to 1991. But the while( time(0L) == 0 ) is a WTF no matter how you look at it.

  • Andrew (unregistered)

    I have to break with the idea that the "structure" is a WTF, how it's implemented surely is. Having entered into kernel programming, I have to say my strict no goto at any cost is relaxed. It just doesn't make sense, and judiciously implementing them is good and makes for quite readable code. However, that said, their use does rather fall away with OO and well done exceptions. With regard to the do { ... } while (false); construct is definitely a brain twister when first encountered, when read properly, it actually makes sense and can be used quite cleanly. Obviously, it's not a loop, the condition is never true so the body is executed but once. Thus, one reads the instructions as, "Do .. and ... and ..." and at any point if a problem occurs, stop (i.e. break), properly clean up whatever is necessary and return the code. In languages that don't use exceptions or try/catch/finally, it's not a bad solution.

  • (nodebb) in reply to sizer99

    Please say that the bool FILE_NOT_FOUND is a joke 😧

  • Adam (unregistered)

    OP here. The code is definitely C++, although the snippet submitted could easily pass for C code (C99 has a bool type from stdbool.h, as some folks have noted). It's inside a class member function named WavFileReader::Open() and does some other unnoteworthy things like access class member variables.

    The use of a 5-byte null-terminated buffer with strcmp() instead of memcmp() is a minor WTF but not a bug.

    The function returns the value of 'ok' on completion. When it fails to parse for any reason, the caller has no idea why, just that loading failed. No error codes or exceptions are used to differentiate between failure modes, and there is zero logging of any kind.

  • JJ (unregistered) in reply to Sole Purpose Of Visit

    I'll help you rein it in. Exceptions are not in my '86 -Bjarne1.0 printing, but in the '97-Bjarne+11. Just checked the latter, SEH is not a term used. Possibly MS invention.

  • Spike (unregistered) in reply to R3D3

    https://thedailywtf.com/articles/What_Is_Truth_0x3f_

  • Worf (unregistered)

    I've seen do { ... } while (false); as a C idiom, almost always because using braces isn't portable (thus you often see it used in a macro where you need to scope things).

    Of course, modern C allows you to sub-scope things by sticking an empty brace on a line and close it further down (it's what do { .. } while(false) is doing). But ancient compilers are still a thing and they can barf when they unexpectedly hit a brace.

    I've never seen it abused as GOTO though. Though use of GOTO as a structured way to exit from a function to avoid heavy nesting isn't unusual, especially if you need to clean up after yourself. But this is C idioms. C++ you're supposed to ditch that, that's why we have structured exception handling so you don't have to check return values after every function.

  • jgh (unregistered) in reply to {Error:GET_NAME_FAILED}

    I would have removed the header test out of the loop where it doesn't belong, leave the loop where it is.

  • jgh (unregistered) in reply to Kleyguerth

    I've just spent the afternoon changing a light fitting. It took four times longer than I expected because the previous sparky had a) used a ceiling rose as a junction box and b) pulled all the cables tight leaving no slack. I spent four hours with my head in the ceiling adding two junction boxes onto five sets of cables to give me a spur to connect the replacement fitting to. I've now got it to the state where I can replace the light fitting.

  • (nodebb)

    Should have just returned a null.

    On another note, was the writer trying to break the compiler's attempts to optimise the flow by using a volatile test liketime(0L) == 0?

  • MiserableOldGit (unregistered) in reply to Jaime
    Try designing one from scratch, without copying an existing one, and see if you can do it first try.

    I can if I follow the guidance spelled out in electrical codes and safety standards. Also, that industry requires not only compliance with standards, but actual testing (in the case of products) or inspection (in the case of installation). In the software industry, we have been unable to achieve reliability and/or security even on systems where that is a stated goal. We struggle with testing and inspection and half of our industry outright rejects the necessity of either.

    I'd invite you to give it a go and report back.

    I come from the construction industry to this sordid game, in that field of engineering cocky newbies are always given some "simple" task to design so they can be ritually humiliated about how what they produced couldn't be assembled or constructed or would fall apart as soon as someone tried. It's just not as simple as you think, even for a light fitting, unless you copy a working example.

    A bit like our industry standard of asking someone new to recurse through a directory tree or produce a regex to validate an email or something. We should be able to give the answer, or know to confidently say "piss off".

    The problem is that with software shit ships, and there's no refunds.

  • löchleindeluxe (unregistered)

    The only thing worse than a Perl programmer writing Perl is a Perl programmer writing C. And frankly, this looks exactly like the kind of block-break-look-at-$¤ they do once the Stockholm Syndrome catches up with them.

  • Chris (unregistered) in reply to Brian

    I see that they didn't initialise the whole array, they just used char id[5];. I believe in release mode this will be initialised to 0's, but in debug it will be initialised to 0xCD's (Windows / MSVC thing apparently). Explicitly stating id[5] = {0} would be better, but since they're going to fill the first 4 bytes from the file anyway, it's unnecessary.

    We've been left in the past scratching our heads wondering why two large files created by some code were different between Debug and Release (noticeable by the file signatures being different), and eventually worked out the some array cells were uninitialised and never filled in / used.

  • Edd (unregistered)

    TRWTF with strcmp is when they hear it's bad, so they get the length with strlen and use it with strncmp.

  • Herr Otto Flick (unregistered)

    Many places "back in the day" distrusted the overhead of C++ exceptions, all the places I worked in the noughties doing C++ development would routinely compile -fno-exceptions to disable them entirely.

  • Goto C (unregistered)

    It does look like this is C code that's been wrapped or pasted into a C++ library. Returning error codes is classic C and not a WTF, just legacy - although not returning a different error code for each case is a bit of a WTF because 'couldn't open file' vs 'file is not a RIFF' vs 'file claims to be a RIFF but is invalid' are all different things that I might like to know, as a caller.

    If the calling code is expecting an error code then throwing an exception breaks the contract, and throwing an exception just to catch it and return an error code would be a bigger WTF than the code shown.

    do { ... } while(false) with breaks to avoid using goto doesn't seem like a WTF to me - it's well controlled, the jumps are local and it's pretty easy to see what's going on.

  • sizer99 (google) in reply to R3D3

    It is a joke, reffing this classic WTF :) https://thedailywtf.com/articles/What_Is_Truth_0x3f_

  • xtal256 (unregistered) in reply to Jaime

    I was listening to the radio the other day, where the presenter was talking to someone via Zoom or something. They commented, "I love it when technology works!". I thought to myself, imagine saying that about something like a car: "I love it when my car starts without any problems!". How silly it would sound. People expect everything in this world to just work - except software. That they almost always expect to break.

  • xtal256 (unregistered)

    As if to prove my point, newlines in my above post were eaten.

  • David Mårtensson (unregistered) in reply to Jaime

    Installing a light fixture is like modifying one small function, writing anything more complex is like installing a stair switch with turnof timer for 10 floors along with motion sensors and emergency lighting, in which case you would also be quite happy if everything works fine right out of the box.

    Software is much more complex in general than many other problems with many more possible problems.

    A good worker would probably get it to work right away, but once in a while when you do something you do not do on a regular day you will make mistakes :)

  • MiserableOldGit (unregistered) in reply to xtal256
    "I love it when my car starts without any problems!". How silly it would sound.

    I'm old enough to remember when that was the exception rather than the rule, especially on a cold morning. Mind you, that was Britain, and in the late 70s, early 80s a new car was usually only just about capable of limping out of the showroom to your house before breaking down.

  • Rohan (unregistered) in reply to Watson

    I think you may be onto something. There's a similar anti-optimisation (pessimisation?) trick in widespread use on Shadertoy. Something along the lines of: #define ZERO min(0, iframe) and using that whenever a test against zero is needed.

    In that case, it's to prevent the compiler from inlining a big complicated function that gets multiple calls and ballooning the resulting compiled shader code.

Leave a comment on “The Sound of GOTO”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article