• Abso (unregistered) in reply to derp
    derp:
    This code is obviously going into a million+ units of embedded car ECUs, [...]
    Then why would they hand it to a single developer who was totally unfamiliar with it for "upkeep 'n such"?
  • Moebius Wilburs (unregistered) in reply to Nagesh
    Nagesh:
    Columbus sailed the ocean blue, (forget the rest)
    ...looking to steal from you, Instead he found the Caribean, and raped the Red Indian.
  • (cs) in reply to Moebius Wilburs
    Moebius Wilburs:
    Nagesh:
    Columbus sailed the ocean blue, (forget the rest)
    ...looking to steal from you, Instead he found the Caribean, and raped the Red Indian.

    Huh!!! I never heard that before...

  • (cs) in reply to Nagesh
    Nagesh:
    Moebius Wilburs:
    Nagesh:
    Columbus sailed the ocean blue, (forget the rest)
    ...looking to steal from you, Instead he found the Caribean, and raped the Red Indian.

    Huh!!! I never heard that before...

    That's because he just made it up, and it's freakin badass!

  • (cs) in reply to Moebius Wilburs
    Moebius Wilburs:
    Nagesh:
    Columbus sailed the ocean blue, (forget the rest)
    ...looking to steal from you, Instead he found the Caribean, and raped the Red Indian.
    Is this the real Moebius Wilters? I love that guy!
  • (cs) in reply to Moebius Wilburs
    Moebius Wilburs:
    Nagesh:
    Columbus sailed the ocean blue, (forget the rest)
    ...looking to steal from you, Instead he found the Caribean, and raped the Red Indian.
    Is this the real Moebius Wilters? I love that guy!
  • NOT the original, ORIGINAL by (unregistered) in reply to hoodaticus
    hoodaticus:
    Moebius Wilburs:
    Nagesh:
    Columbus sailed the ocean blue, (forget the rest)
    ...looking to steal from you, Instead he found the Caribean, and raped the Red Indian.
    Is this the real Moebius Wilters? I love that guy!

    Wow, a double post... Now that's a LOT of love.

  • Hungrybear9562 (unregistered) in reply to NOT the original, ORIGINAL by
    NOT the original:
    hoodaticus:
    Moebius Wilburs:
    Nagesh:
    Columbus sailed the ocean blue, (forget the rest)
    ...looking to steal from you, Instead he found the Caribean, and raped the Red Indian.
    Is this the real Moebius Wilters? I love that guy!

    Wow, a double post... Now that's a LOT of love.

    What does it mean?

  • (cs)

    I'll join the contingent that agrees this kind of system is perfectly acceptable and normal, especially in embedded systems.

    Two changes I would make to make it less WTFy:

    1. Typedef the function pointer type to make the code more readable. Function pointer semantics are a black art to a lot of people to begin with, so declaring an array of them without a typedef isn't being particularly kind to those who aren't C veterans.

    2. Change SID[MAX_NUM_SERVICE_IDENTIFIERS] to just SID[], and add a static assert (compile-time assert) that the size of the SID array == MAX_NUM_SERVICE_IDENTIFIERS. Why? Because just putting MAX_NUM_SERVICE_IDENTIFIERS as the array size doesn't guarantee that you'll have initialized the correct number of values. Instead, it's a far better practice to let the compiler size the array based on what you've entered, and then have it assert that the size it arrived at matches your expected value. That way if either the array contents change or MAX_NUM_SERVICE_IDENTIFIERS changes it will be caught on the next build attempt.

    For bonus points, I would replace the NULLs with a function that asserts in debug builds.

    Dan.

  • MarkG (unregistered)

    Well,

    It's not the most ideal way to do it but it isn't unheard of. It's for processing messages for automotive protocols. I'm willing to bet that start_diagnostic_session is at position 0x10 and security_access is at position 0x21.

    The RWTF here is that you should understand the nature of the thing you are maintaining. I'm not saying a table full of NULL pointers is the best approach -- but I could see how it could be useful. Especially if there are multiple tables like this and the current state or mode of communication adjusts the table used.

    Having worked on this kind of stuff I tend to use switch/cases for these since the messages you deal with are sparse.

  • (cs) in reply to dposluns

    Or my C99 version above, which will initialize exactly the fields which are meaningful to initialize, and set the rest to NULL. Trouble is the lack of C99 in the embedded C market.

    (Does this thread belong on comp.lang.c instead?)

  • (cs) in reply to Abso
    Abso:
    derp:
    This code is obviously going into a million+ units of embedded car ECUs, [...]
    Then why would they hand it to a single developer who was totally unfamiliar with it for "upkeep 'n such"?

    I doubt that's what happened. More likely, the submitter went poking through the codebase, found this thing, didn't immediately have a clue what it was for, and assumed it must therefore be a WTF.

    What makes it doubly hilarious is that this code was most likely automatically generated, rendering any maintenance argument moot and making the suggestions here of how to make it more "maintainable" look completely ridiculous.

  • an (unregistered)

    TRWTF is the lack of typedefs, useless use of ampersand and sparse array initialization.

    typedef void (*sid_callback_t)(const u8 * data, const u16 bytes);
static const sid_callback_t SID[MAX_NUM_SERVICE_IDENTIFIERS] = {
        [16] = start_diagnostic_session,
        [26] = read_ecu_identification,
        [34] = rd_data_by_common_identifier,
        [39] = security_access,
        [46] = wr_data_by_common_identifier
};

    Captcha: eros -- C is only for real men.

  • (cs) in reply to Daniel
    Daniel:
    Forgot to mention: not necessarily precisely ISO 14230 Keyword Protocol 2000 but possibly a harmonised spec such as Swedish Implementation Standard (SSF) 14230. Either way, it's the same thing in different clothes.
    I'm amazed at how many ECU C programmers we have here. Lurking. Waiting to kill us all with their leetness.

    Seriously, I'm glad they're here.

  • (cs) in reply to boog
    boog:
    Nagesh:
    hoodaticus:
    His use of "Indian" was a play on words.
    In that he should have just used the term "Red Indian". That's we Indians usually understand very quickly.
    Then it wouldn't have been a play on words. </duh>
    Now, if we were actually racist against East Indians, we'd replace "casino" in the prior example with, say, "7-Eleven", and think it more accurate.
  • (cs) in reply to Guest
    Guest:
    It really is like I'm looking at the Win32 API!
    Welcome to the world before objects. What you see before you is the primordial ooze whence they evolved.
  • Stuart (unregistered) in reply to MarkG
    MarkG:
    The RWTF here is that you should understand the nature of the thing you are maintaining.
    Absolutely. I've never worked in the embedded space, or on automotive systems, but I do know C, and after the initial two seconds of "WTF is that" had passed and I actually looked at it, it was very clear it was a dispatch table based on decoding a protocol of some sort. It certainly could have done with more comments, but it's absolutely a valid approach that a real C programmer will understand.
  • (cs)

    All I can say is, "NULL, NULL, NULL. NULL, NULL, NULL and NULL."

  • SP (unregistered) in reply to dposluns
    dposluns:
    For bonus points, I would replace the NULLs with a function that asserts in debug builds.

    I expect that if SID[id] is NULL the calling code sends a "service not supported" response.

  • Not of this Earth (unregistered) in reply to SP

    I got it! TRWTF is Johnny D bitching about pretty standard scalable solution!

  • Design Pattern (unregistered) in reply to smxlong
    smxlong:
    Abso:
    Then why would they hand it to a single developer who was totally unfamiliar with it for "upkeep 'n such"?

    I doubt that's what happened. More likely, the submitter went poking through the codebase, found this thing, didn't immediately have a clue what it was for, and assumed it must therefore be a WTF.

    What makes it doubly hilarious is that this code was most likely automatically generated, rendering any maintenance argument moot and making the suggestions here of how to make it more "maintainable" look completely ridiculous.

    The question still remains why he was poking through the codebase, unaware of the development process / code generation.

    Handing over development/maintenance to an incompetent programmer not knowing standard C practices and not giving him information about the code generation is TRWTF.

  • (cs) in reply to aks
    aks:
    I also don't see the WTF. A function pointer dispatch table is anyday better than a giant fugly switch(){ case : } or a huge if () else if(). Not to mention better maintainablity and way way way more faster than those comparisions can ever dream of becoming (think pipelining on ARMs). Plus I doubt that somebody wrote that by hand, it probably was generated using a tool (or Excel, which is excellent for maintaining such arrays and readability). I guess that the author has not found the tool (or the excel sheet) to do so and is bitching about it.
    If you're right that it was in fact created by a tool (and I have already asserted that it *should be*, so I'm in full agreement with the concept), there are two small WTFs.
    1. The tool doesn't include in its output a comment to help the maintainer find it.
    2. It seems highly likely that someone checked the generated code into version control rather than marking it as ignorable.
  • Guest (unregistered)

    I really liked this article and comments keep up the good work. Researching function pointers was a very educational way to spend a slow Friday afternoon at work.

    Looks like he is declaring an array of pointers SID. Where the majority of the contents are NULL pointers and the non NULLs are the addresses of functions.

    To me the WTF is the risk of de-referencing a NULL pointer.

  • Joe (unregistered) in reply to an
    an:
    TRWTF is the lack of typedefs, useless use of ampersand and sparse array initialization. 
    typedef void (*sid_callback_t)(const u8 * data, const u16 bytes);
static const sid_callback_t SID[MAX_NUM_SERVICE_IDENTIFIERS] = {
        [16] = start_diagnostic_session,
        [26] = read_ecu_identification,
        [34] = rd_data_by_common_identifier,
        [39] = security_access,
        [46] = wr_data_by_common_identifier
};
    Captcha: eros -- C is only for real men.

    I was about the post almost identical code, until I saw your post. The only change I would recommend is that the literal subscript values should probably be replaced with the enum values identifying the opcode number or message number (depending on what type of dispatch table this is) that will be used when actually indexing this array.

    One should mention that your code is C99, and thus if for some reason you absolutely must use a C90 compiler, it would not work.

  • anon (unregistered) in reply to hoodaticus
    hoodaticus:
    I do this same thing in C#. After all, the simplicity of using lambda expressions is made even more simple by passing them around in giant STRUCTS. I dump every lambda I need into a struct and just pass it around everywhere.

    Well, aren't you just the coolest.

  • anon (unregistered) in reply to NOT the original, ORIGINAL by
    NOT the original:
    Jenkins:
    NOT the original:
    DOA:
    NOT the original:
    foo:
    NOT the original:
    Man, that gets boring really fast... I'm curious what the payload-to-troll content ratio currently is. I'm pulling a magic out-of-my-ass number and say it's at least 50/50.
    At least? So you mean, there's 50 percent or more useful content? I thought it was more like 10/90 or less.

    Hey now, lets not get out of hand here. Nobody said useful content, I just said at least 50% is non-trolling. This could mean actual useful content or someone smashing their fish on a keyboard for 10 seconds before hitting the submit button.

    hndsafhflk;hds;lfkhodsioudsuhniks ujmdfpdsisd098u9dfsa-]9sadfs9yh[0fsasa bsda0 f dsfba

    See! We need moar of this! ^^

    You mean sock puppeteering?

    Unfortunately, the end result of too much trolling is that everybody thinks that everybody else is trolling, always. It's like the whole forum has gone paranoid-schizophrenic. Hearing and seeing trolls when there isn't any, etc.

    (In case I wasn't clear enough, I'm not sock-puppeteering)

    If you say so Mr. Hat.

  • anon (unregistered) in reply to Nagesh
    Nagesh:
    hoodaticus:
    Nagesh:
    Lingerance:
    Nagesh:
    Nagesh:
    ¢ÃƒÆââ‚Â:
    Nagesh is TRWTF.

    And you call us racist for saying outsourced code from India sucks?

    You are TWRTF and you are racist. Our code is much better than any USA code you can name, I dare you to prove different. Our last client paid lots of money for our code and they were so happy they got a faster server to run our code. I also wrote 6 thousand lines of code yesterday. I would have coded more but my keyboard is not working well, the keys do not always work so I ut and paste with the mouse.

    Why you force client to buy new server? OUr client still running our super fast quick code on 486 and Pentium machine. He never complain. Truly good code is one that runs on less resources.

    I'm still not sure why we let Indians write code anyway. Is it some sort of reparations? Isn't giving casinos to you enough?

    What casinos? Gambling is illegal in India.

    True story from Mahabharat. Look up Yudishthira, Draupadi, Gambling, Dice, Shakuni, Pandav.
    His use of "Indian" was a play on words. He meant MY kind of Indian, not your kind of Indian. Your kind of Indian is what Christopher Columbus was looking for when he discovered America for the Europeans. MY kind of Indian is what the Europeans actually found when they got here.

    You're East Indian. I'm West Indian (part Cherokee). Both are swarthy heathens, but your kind didn't chop the scalps off of the invading British horde for trophies. You also probably didn't eat entire British colonies, which is my educated guess as to what happened to Jamestown.

    I bet they were delicious.

    In that he should have just used the term "Red Indian". That's we Indians usually understand very quickly. Thanks for the clarification, though.

    Columbus sailed the ocean blue, (forget the rest)

    Nagesh, you are very racist, biggotted, small minded, and xenophobic for referring to native americans as "Red Indian". BTW code from India still sucks.

  • JoC (unregistered)

    The code is self-documenting.

    You count NULLs on each line and convert the numerics to the equivalent ASC chars to get a short succinct comment.

  • (cs)

    It's just NULLs, all the way down.

  • J.D. (unregistered) in reply to NOT the original, ORIGINAL by

    Yes, actually, I really had no idea what it was doing. And neither did much anyone else in the company. Finally I found a guy, who told me how that structure works. But that still doesn't explain, why there are 255 cells being used? Wouldn't just five cells be enough? Or, like suggested in an earlier post; five if's or a switch-case?

  • J.D. (unregistered) in reply to smxlong
    Abso:
    derp:
    This code is obviously going into a million+ units of embedded car ECUs, [...]
    Then why would they hand it to a single developer who was totally unfamiliar with it for "upkeep 'n such"?
    Actually, you are now at the heart of the WTF. We've had some people leave the company and I've been used as the tossing bag between random, undocumented projects, that just need someone to make them do what they're supposed to do.
    smxlong:
    I doubt that's what happened. More likely, the submitter went poking through the codebase, found this thing, didn't immediately have a clue what it was for, and assumed it must therefore be a WTF.
    In this case there was a bug in one of those functions that are beyond the vtable. I personally hadn't encountered anything like this before and had no idea how it does what it does. And in any case, there have been more useful ways within these comments to implement this, than to make a complete 255 cell NULL table.
  • Max E. (unregistered)

    I think this code looks reasonable... not great, but reasonable. I probably would have used a switch statement with an enum here, but I'm not an embedded programmer and I can see why this would be more reasonable in an embedded context.

    However, if this code is machine-generated, then the input files for the code generator should have been the "real" source code, with the snippet being an incidental part of the build process. So TRWTF is how did Johnny D even see this code!

  • Tiago Cardoso (unregistered) in reply to Nagesh
    Nagesh:
    Inside each small problem is a bigger one waiting to get out.
    Best comment I've ever seen here :D
  • Anonymous (unregistered) in reply to frits
    frits:
    Who hasn't done something like this?

    Java programmers.

Leave a comment on “The Interface”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article