- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Admin
Filed under: Sarcasm Self-Test: Complete.
Admin
No, it's not useful. Not if the API doesn't actually call it.
Admin
Look at the bright side - at least the self-test never fails!
Admin
strcpy() in embedded code? Sure! Everyone is fully aware of the maximum length of the array that's passed and the maximum length of the string that can be written in it. And that number will never change.
Admin
Oh my god, it's like gets() all over again!
Admin
Let's hope, for the sake of our credit cards, that "PCI", in this case, means Peripheral Component Interconnect, and not Payment Card Industry.
Of course, it could also mean Parti Communiste Internationaliste, but it's not May first yet.
Admin
Mild WTF in the self test function, and in fact in the whole API...
API_SUCCESSis not zero.Why is this a WTF?
How many ways can something succeed? Usually(1), just one.
How many ways can something fail? Usually, an unbearable multitude.
If success is NOT zero, how many ways can we report success? An unbearable multitude. And failure? Just one.
There's a moderate disparity there.
(1)
S_FALSE, I'm looking at you here when I say "usually". It's a#definesuccess return value that you can use in anHRESULTreturned from a COM-called function.FAILED(S_FALSE)will return FALSE.S_FALSE, however, is defined as 1. False, 1, what can go wrong there? Bigger WTF:S_FALSEisn't just an ugly face. Some COM-related functions will return it under certain circumstances. It is why, in general, you should never ever no not ever just testHRESULTvalues to see if they are not zero as a failure test.Admin
Wouldn't it be nice if some programming language came up with the idea of a boolean type rather than abising integers in stupid ways? That would be magical.
Admin
Actually, since API_SUCCESS is 0, the part that passes out "Test Successful" is never called! If you go by the C-string out param, this is a self test that never passes!
Admin
I am sure people will always have appreciated how fast the self test is completed. :grinning:
Admin
And quite often, subtle and difficult-to-diagnose bugs will be introduced yet not detected.
Fixed! :smiling_imp:
Admin
Judging by the names and signatures of these functions, yes, actually, the maximum length that can be written to the string is mandated by the standard. Also, I don't think this is embedded code, but rather code from the host driver to control the instrument, which itself probably runs on Windows.
Admin
The fact that you correctly deduced that standard from slightly-anonymized code is terrifying.
Admin
Let's just say that the function names were a bit of a give-away.
Admin
Whether it's embedded or not is irrelevant. This is a buffer overflow bomb just waiting for a programmer who doesn't know he's supposed to count the characters in that literal string, doesn't care, or makes a mistake. Or a programmer who's working on the calling functions and notices he's running short on stack space and decides to shorten that response buffer a little bit.
Admin
Or you could use Perl. That way you get to abuse all types, not just integers :smiley:
Admin
Absolutely, yes. Welcome to the land of C strings.
Admin
Obviously, the right thing to do is to
bzero()the return buffer out to its expected length before doing astrcpy()into it. That'll make sure that everyone who fucks up at least gets to notice it at once…Admin
Well in most cases that would be correct, but in case your buffer-overrun is only a few bytes, only the base pointer gets clobbered with 0x0h and not the return address (which would result in the program having problems).
I'm unsure what a clobbered base-pointer in a stack frame would result in, but it is likely there for a reason but it can (in at least some cases) be clobbered without immediate program termination (refer computerphiles vid on buffer overflow attacks)
Admin
Having seen that sort of thing, it can result in the weirdest crashes. In the debugger…
Admin
If I saw this in a codebase I was working on, my first thought would be that this was meant to be a skeleton for the actual, device-specific code, but that the original coder failed to document that (and add it to the IDE's auto-stubbing function, if it has one - and if it doesn't, that a :wtf:, too, since it is such a trivial feature to implement and really useful for cases of repeated code - at least when you don't have templates, generics, and/or lexical macros in your language, that is...).
Admin
My first thought was that these were dummy, no-op implementations (
error_queryandrevision_query, that is) which would get overridden by the actual driver implementations. On their own, they're not that much of a :wtf:, aside from the usestrcpy().Admin
Or JavaScript.
Admin
Do you work for Volkswagen ?
Admin
No surprise. I've seen RTL code shipped that clearly was never actually used. This was in the protected mode era, the code manipulated real-mode pointers as protected mode--thus causing a very high probability of a segment fault. The debugger had a bug, though--in single-stepping mode an invalid segment register load would be executed anyway. (Apparently single-stepped instructions were being emulated.) Thus the code would work perfectly when single-stepping in the debugger but would blow otherwise.
Admin
I think you want an enum here instead of a bool. Which, oddly enough, C supports.
Admin
In the COM case that @Steve_The_Cynic mentioned, it's a little different. You have more than one type of errors and more than one type of successes. Hence the SUCCEEEDED() and FAILED() macros on the HRESULT make sense. (S_FALSE is a successful boolean return value)
Admin
Admin
Further blame reveals this diff:
unsigned long retval = API_SUCCESS; *testResult = 0; -- retval = device->doSelfTest(); ++ // retval = device->doSelfTest(); if (retval) { strcpy(result, "Self Test Successful"); }Along with the commit message:
Admin
Hmmn, looks familiar, but I doubt that the guy I knew ever worked professionally in anything other than ShillFarce...
Admin
...and respond by removing the buggy and unnecessary
bzero()call.Admin
Frankly, I kind of get the idea behind
S_FALSE, but I've still learned never to write COM code that returns it, if only because several client languages have no concept of multiple successes: Either the method returns, or it throws an exception. And some such client languages don't even have a[PreserveSig]attribute to work around the problem.Admin
I laughed out loud. I probably would not have laughed if I had been the one to find that commit in a project I was involved in though. Any idea what happened to highly paid contractor?
Admin
cashed a fat bill and ran away ?
Admin
This would be appropriate (but unlikely, unfortunately): https://en.wikipedia.org/wiki/Hanged,_drawn_and_quartered
Admin
He is commissioned to fix regressions, that line may need to be commented again.
Admin
It could be a Peripheral Component Interconnect card plugged into the system implementing Payment Card Industry.
Admin
Noothing to do with it. They didn't fuck up anything, they knew exactly what they did.
Admin
I believe that's the reason why in batch languages, the return value 0 means success and any non-zero return value a failure of some kind. (Unless the specs say it carries some other type of information.)
Well, it's easy to teach a preprocessor to replace an enum by a bundle of const. Abusing integers as a kind of boolean is error prone - you never know if the compiler wouldn't find it funny to take ! as a bit-wise operator.
Which gives me a nice opportunity to mention one of my favorite WTFs here - http://thedailywtf.com/articles/A-Smorgasbord-of-Classics the one by Christophe Beugnet (last entry).
Admin
When revenue started going down, the tester of the test routines was the first one to be laid off.
(And yes, :minidisc::horse:, I know answering to different subtopics in different posts is Doing It Wrong™.)
Admin
Admin
Well, shell programs can return two streams of values and you can open more if you want.
Admin
That was realised long ago. There's been a lot of disagreement about how to do it.
Admin
Just so - it's an issue people have been arguing over since the mid-1960s, if not earlier. Interestingly, a number of RISC CPUs define multiple return value registers in their calling conventions, with MIPS using two ($v0 and $v1), ARM-32 four (r0-r3, which are used for both argument and return), ARM-64 nine (x0-x7, which as before double up passing and return, plus x8 for pointers to larger return values), and SPARC eight. So this is not only a known issue, is it one addressed in hardware (well, by convention anyway) by most systems since the early 1980s - just not the one most widely used for desktop and notebook systems, go figure.
All well and good for assembly programming, sure, but trying to have a high-level language use that feature without excessively tight coupling to the hardware? Not likely to happen. In practice, using multiple return registers tends to fall into the category of 'compiler optimization' rather than 'language feature' (e.g., register painting being applied to returning a compound structure to avoid pointer indirections).
But yes, the biggest issue in handling multiple return values in high-level languages has been how you pass them back, especially across different languages. For example, Python represents multiple returns as a tuple - a data type few other languages have, and which can be implemented in several different ways even by different Python interpreters. Various Lisp languages have had multiple return values (and returning several values as a list is always an option), but none of them agree on how to implement the feature, even for different implementations of the same dialect. Icon (the SNOBOL-descended string manipulation language, not the visual language of the same name) has multiple return values and every function returns a specialized, out-of-band success/failure value as well. Similarly, logic languages like Prolog usually have a backtracking feature, which behaves in some senses like multiple returns (even though it is quite different conceptually).
Admin
Return multiple values is great, I use that in Python a lot. But returning error code that way is even more :wtf: because you say, hey here is the answer you were looking for but here is the error :laughing: make things too paranoid and fuzzy.
Exceptions are OOB channel for errors, and they are perfect.
Admin
True, but it depends on what you consider to be an exceptional condition. Raising and catching an exception is an expensive proposition given the more common implementations, so you want to avoid exceptional conditions whenever possible. Raising an exception should mean that something really unexpected has happened, something that the code where the exception occurred lacks the context to handle gracefully. Exceptions shouldn't be raised unless you aren't sure how to deal with them, and have no way of getting that contextual information from somewhere else so that you do so.
What does this mean? It means that exceptions are a backstop for emergencies, not for ordinary flow of control. They are one tool in the toolbox, but not one which needs to be used every time.
So, how do you handle things without exceptions or passing back an error code? Well, if a given condition is known to be likely, but requires additional context to handle, you need to have some way to pass that context to the method - for example, through dependency injection, or by passing a higher-order function (which amounts to almost the same thing), or for more complicated cases, through a context registry. If you can tell the method how to deal with the not-quite-exceptional cases beforehand without raising an exception, and reserve exceptions for the real 'what the hell just happened?' cases, it should make the exception handling cleaner (at the cost of some messiness in the object interface, admittedly).
Admin
Not really. It's computing the stack trace that is hellishly expensive. If there's no stack trace or allocated payload, an exception need not be very expensive at all, not much more than a returned error code. (The RAII unwinding might cost, but that's something else entirely…)
Admin
True, but a lot of implementations build a stack trace by default (and pretty much all do when there is no explicit exception frame to catch the error, though at that point efficiency is a moot issue), or at least that used to be the case.
In any case, there are design and refactoring reasons for using DI anyway; it splits the burden of handling the undesirable-but-not-exceptional state between the class implementor and the client programmer in a way that is likely to serve the purposes of both better, and can make the error handling smoother (because the issue often can be handled in place without disrupting flow of control and without having to back out of the code where the error occurred).
Another option for avoiding that particular problem (backing out, I mean) might be to include a continuation in the exception payload, but for most languages adding continuations is really, really not an option.
Admin
Well, that's how Go does it (not that anyone here likes Go). And it's the most concise way I can think of:
value1, value2, errorcode = function(x); if (errorcode != 0) { [handle error] };. I was mostly thinking about shell scripts anyway.I suppose the semantically correct way to describe that is to say the function can return either an error or a result, and each of those can have more than one variable/value/element (error type, error subtype, whatever). But I can't even imagine how you'd type a function call like that.