• (disco)
  • (disco)

    Well surprise surprise.


    Filed under: Sarcasm Self-Test: Complete.

  • (disco)

    One of the firmware functions is an internal self-test which is useful for verifying that new firmware versions function or that hardware isn’t physically damaged.

    No, it's not useful. Not if the API doesn't actually call it.

  • (disco)

    Look at the bright side - at least the self-test never fails!

  • (disco)

    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.

  • (disco)

    Oh my god, it's like gets() all over again!

  • (disco)

    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.

  • (disco)

    Mild WTF in the self test function, and in fact in the whole API...

    API_SUCCESS is 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 #define success return value that you can use in an HRESULT returned 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_FALSE isn'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 test HRESULT values to see if they are not zero as a failure test.

  • (disco) in reply to Steve_The_Cynic

    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.

  • (disco) in reply to sloosecannon
    sloosecannon:
    Look at the bright side - at least the self-test never fails!

    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!

  • (disco)

    I am sure people will always have appreciated how fast the self test is completed. :grinning:

  • (disco)

    And quite often, subtle and difficult-to-diagnose bugs will be introduced yet not detected.

    Fixed! :smiling_imp:

  • (disco)
    Hanzo:
    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.

    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.

  • (disco) in reply to ixvedeusi

    The fact that you correctly deduced that standard from slightly-anonymized code is terrifying.

  • (disco) in reply to mott555

    Let's just say that the function names were a bit of a give-away.

  • (disco) in reply to ixvedeusi

    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.

  • (disco) in reply to Jaloopa

    Or you could use Perl. That way you get to abuse all types, not just integers :smiley:

  • (disco) in reply to RogerC
    RogerC:
    This is a buffer overflow bomb just waiting

    Absolutely, yes. Welcome to the land of C strings.

  • (disco) in reply to RogerC
    RogerC:
    This is a buffer overflow bomb

    Obviously, the right thing to do is to bzero() the return buffer out to its expected length before doing a strcpy() into it. That'll make sure that everyone who fucks up at least gets to notice it at once…

  • (disco) in reply to dkf

    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)

  • (disco) in reply to Yazeran
    Yazeran:
    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)

    Having seen that sort of thing, it can result in the weirdest crashes. In the debugger…

  • (disco)

    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...).

  • (disco) in reply to ScholRLEA

    My first thought was that these were dummy, no-op implementations (error_query and revision_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 use strcpy().

  • (disco) in reply to John_Imrie

    Or JavaScript.

  • (disco)

    Do you work for Volkswagen ?

  • (disco)

    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.

  • (disco) in reply to Jaloopa

    I think you want an enum here instead of a bool. Which, oddly enough, C supports.

  • (disco) in reply to anonymous234

    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)

  • (disco) in reply to mott555
    mott555:
    The fact that you correctly deduced that standard from slightly-anonymized code is terrifying.
    That's why TDWTF has historically preferred to anonymise code so heavily that it's impossible to determine even the general nature of the original.
  • (disco)

    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:

    Fixes the problem with self test not passing.  (Signed-By: highly paid contractor)
    
  • (disco) in reply to dse

    Hmmn, looks familiar, but I doubt that the guy I knew ever worked professionally in anything other than ShillFarce...

  • (disco) in reply to dkf

    ...and respond by removing the buggy and unnecessary bzero() call.

  • (disco) in reply to Steve_The_Cynic
    Steve_The_Cynic:
    It is why, in general, you should never ever no not ever just test HRESULT values to see if they are not zero as a failure test.
    Yup, that's what the `SUCCEEDED()` macro is for, so people don't make this kind of mistake.

    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.

  • (disco) in reply to dse
    dse:
    Fixes the problem with self test not passing.

    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?

  • (disco) in reply to na5ch

    cashed a fat bill and ran away ?

  • (disco) in reply to na5ch
    na5ch:
    Any idea what happened to highly paid contractor?

    This would be appropriate (but unlikely, unfortunately): https://en.wikipedia.org/wiki/Hanged,_drawn_and_quartered

  • (disco) in reply to na5ch
    na5ch:
    Any idea what happened to highly paid contractor?

    He is commissioned to fix regressions, that line may need to be commented again.

  • (disco) in reply to ka1axy

    It could be a Peripheral Component Interconnect card plugged into the system implementing Payment Card Industry.

  • (disco) in reply to fievel000

    Noothing to do with it. They didn't fuck up anything, they knew exactly what they did.

  • (disco) in reply to Steve_The_Cynic
    Steve_The_Cynic:
    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.

    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.)

    anonymous234:
    I think you want an enum here instead of a bool. Which, oddly enough, C supports.

    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).

  • (disco) in reply to TheCPUWizard
    TheCPUWizard:
    And quite often, subtle and difficult-to-diagnose bugs ***will be introduced yet not detected***.

    Fixed! :smiling_imp:

    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™.)

  • (disco) in reply to PWolff
    PWolff:
    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.)
    Just think of how easier some things would be if people had realized earlier that hey, maybe code should be able to return more than one value.
  • (disco) in reply to anonymous234

    Well, shell programs can return two streams of values and you can open more if you want.

  • (disco) in reply to anonymous234
    anonymous234:
    Just think of how easier some things would be if people had realized earlier that hey, maybe code should be able to return more than one value.

    That was realised long ago. There's been a lot of disagreement about how to do it.

  • (disco) in reply to dkf

    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).

  • (disco) in reply to anonymous234
    anonymous234:
    Just think of how easier some things would be if people had realized earlier that hey, maybe code should be able to return more than one value.

    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.

  • (disco) in reply to dse
    dse:
    Exceptions are OOB channel for errors, and they are perfect.

    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).

  • (disco) in reply to ScholRLEA
    ScholRLEA:
    Raising and catching an exception is an expensive proposition given the more common implementations

    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…)

  • (disco) in reply to dkf

    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.

  • (disco) in reply to dse

    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.

Leave a comment on “The Self Test”

Log In or post as a guest

Replying to comment #:

« Return to Article