• brewbuck (unregistered)

    Are you serious? This is hardly a WTF. Firstly, the comment about strdup() memory being orphaned -- how do you know it isn't freed by code that the submitter did not submit?

    The loop is pretty much exactly how I would do it. The presence of the bogus while() at the end isn't even hard to explain, either -- the code started out as a do-while, then somebody noticed that that wasn't correct if MODULE_PATH was empty, so they changed it to while and forgot to remove the old do-while condition.

    The fact that this amuses you people is baffling to me.

  • Sharkie (unregistered)

    This code snippet is obviously by a senior programmer trying to be completely thread safe.

    It's important to safeguard the value of "path" since between the first loop and second, the value might change to a non-null value.

    This way, if the program gets caught in an infinite loop, you know you have thread-safe issues.

    Yah, yah, that's it!

  • (cs) in reply to Bob
    Bob:
    Dan:
    First!
    Do you shout that after sex too?

    Get real...people that post "frist" here aren't getting any action.

    I empirically verified this by posting "frist" myself a few months ago.

  • AdT (unregistered) in reply to Bob
    Bob:
    Dan:
    First!
    Do you shout that after sex too?

    Sex? We're geeks, you insensitive clod!

  • dkf (unregistered) in reply to Kostas
    Kostas:
    for (String s : System.getProperty("MODULE_PATH").split(":")) { RegisterPath(CreateSymbol(s)); }
    And the code will crash if the enviroment variable does not exist. Congrats, you managed to write worse code than the original.
    Actually, the original code blindly does a strdup() of the result of getenv(), and there is no reason for anyone to assume that strdup(NULL) will consistently return a NULL (in fact, the several versions of strdup implementation I grepped up in a few minutes searching would all keel over sharpish from dereferencing a NULL pointer). So, the Java version does a graceful NullPointerException and the C version dumps core if you're lucky. Guess which one I prefer...
  • AdT (unregistered) in reply to ikegami
    ikegami:
    and given that 
    { ... } while (condition);

    is a single statement,

    These are two statements, a compound statement (aka block) followed by a while statement.

  • dkf (unregistered) in reply to Anon
    Anon:
    It's possible that the 2nd while statement was meant as a comment to indicate what loop the } was terminating. It's not a terrible habit to get into, especially for students, assuming you remember that it's a comment.
    It's not a bad habit at all with very long blocks (and especially 
    #endif
    s). For short stuff, it's just noise. But by making the last bit a trivially satisfied and empty loop, it's Worse Than Failure because it works now but is a terrible booby trap for when someone alters the loop termination condition (either through changing the condition itself or through adding a 'break').

    Indeed, while not being a deep WTF! (I chuckled when I saw it, instead of wanting to claw my eyes out in sheer disbelief) it's a classic Worse Than Failure because of the ticking timebomb it represents; this shows the subtle difference between the two.

  • Tod (unregistered)

    Maybe I'm stating the obvious here - but if I ran across this code I would assume that the original coder simply forgot the comment characters and because the compiler didn't bark and it didn't effect the code they simply never noticed.

    while(path != NULL) { .... } //while (path != NULL);

    Is pasting the command that the last brace closes that uncommon? I admit the snippet is short but it's not a bad habit to mark which brace closes which loop.

    The semi-colon on the end of the statement is the only thing working against this theory. It might have been added to quiet the complier in which case the coder really should noticed the problem.

    =Tod

  • Jonno (unregistered) in reply to brewbuck
    brewbuck:
    Are you serious? This is hardly a WTF. Firstly, the comment about strdup() memory being orphaned -- how do you know it isn't freed by code that the submitter did not submit?

    The loop is pretty much exactly how I would do it. The presence of the bogus while() at the end isn't even hard to explain, either -- the code started out as a do-while, then somebody noticed that that wasn't correct if MODULE_PATH was empty, so they changed it to while and forgot to remove the old do-while condition.

    The fact that this amuses you people is baffling to me.

    Well... I didn't find this one particularly amusing, but I'll bite on the strdup(): First he assigns the result of the strdup to path; then he goes on to assign the result of the strchr to path.. so the pointer to the start of the string is lost. At least that's how it looks to me, but I may well be wrong of course. :-)

  • Jason Scott (unregistered)

    How did we get to 60 comments before someone points out that it's Ken THOMPSON?

  • Federico (unregistered)

    Well, think about the following code:

    int i=10;
{
 printf("Blah\n");
} while(i-- > 0);

    How many times does this write "Blah"? Surprisingly counterintuitive. I wonder if this can be used to hide malicious code, or for obfuscated C contests...

  • (cs) in reply to Jonno
    Jonno:
    brewbuck:
    Are you serious? This is hardly a WTF. Firstly, the comment about strdup() memory being orphaned -- how do you know it isn't freed by code that the submitter did not submit?

    The loop is pretty much exactly how I would do it. The presence of the bogus while() at the end isn't even hard to explain, either -- the code started out as a do-while, then somebody noticed that that wasn't correct if MODULE_PATH was empty, so they changed it to while and forgot to remove the old do-while condition.

    The fact that this amuses you people is baffling to me.

    Well... I didn't find this one particularly amusing, but I'll bite on the strdup(): First he assigns the result of the strdup to path; then he goes on to assign the result of the strchr to path.. so the pointer to the start of the string is lost. At least that's how it looks to me, but I may well be wrong of course. :-)

    It's at least suspicious from what we see. But, there could be another assignment into a temporary variable somewhere, or it could have just been an anonymization artifact (it used to be path0 = strdup(); path = path0; or something).

  • IOCCC (unregistered)

    I win!

  • (cs)

    This CodeSOD has been brought to you courtesy of the Department of Redundancy Department.

  • Brady Kelly (unregistered) in reply to Simon Bradley
    Simon Bradley:
    Why shouldn't it compile? The second loop is a no-op. It has a null statement; and it's always run zero times, since the first loop only exits once ptr == NULL.

    Unless another thread changes sets ptr to a non-null value. Very unfortunate for the debugging coder.

  • s (unregistered) in reply to Corey Miller

    Oh well... Still better than some GOTO avoidances I'd seen.

    jump forward GOTO XXX ... XXX:

    as

    do {

    //kilometers of code

    break; //GOTO XXX

    //even more code

    }while(0); // LABEL XXX

    jump back YYY: ... GOTO YYY

    while(1){ // LABEL YYY

    //kilometers of code

    continue; // GOTO YYY

    //even more code

    break; }

  • London Geezer (unregistered)

    It's not a WTF at all. Just some bad coding. Looks like the second while loop is a typo to be honest or the developer created it before putting the ops in there then went back to it and created another, without deleting the line.

    Never mind, these WTF's are not up to standard lately!!

  • me (unregistered)

    This is little more than a typo. The developer obviously changed his mind and should have:

    a.) change the first 'while' expression to 'do' or b.) remove the second while expression.

    Of course whic of these you choose has subtle effects upon the behaviour of the loop, but the net effect is pretty much the same.

  • Sandy (unregistered)

    Similar code compiles and runs the same way in C#...

        public class Program{
        static void Main(){
            int x = 4;
            while (x > 0){
                x--;
            } while (x > 0);
        }
    }
  • (cs) in reply to TSK
    TSK:
    while - while is actually a possible structure, testing both at the beginning and at the end of the loop (I thought I have seen once at a time a language built with it). In C and C++, it can be simulated by:

    DO WHILE (condA) { (pseudocode) } REPEAT WHILE (condB);

    Wow, whatta bunch of horseshit. First, C and C++ are case sensitive. Second, there's no such keyword as 'repeat' in either of them. Third, there's no such construct as 'do while' without a loop body inbetween the two keywords.

    TSK:
    Don Knuth even mentioned it in one of of his papers as a possible practical structure (it was at a time C was still a young language).

    I'll believe that when you post a link. Sure, it's entirely possible to write

    if (condA) do { ...... } while (condA && condB);

    but it hardly deserves a special language construct. (Hint: a condition at the top of the loop gates initial entry, but after the first iteration it makes no difference if it's at the top or the bottom any more.)

  • crusty old guy (unregistered)

    Doesn't this particular case really equate to:

    while(cond A || cond A) { pseudocode }

  • max (unregistered) in reply to mav
    mav:
    Wow, now THAT code is a genuine WTF. Ugly. Damn ugly.

    Not that ugly, I don't think, except for the fact that this kind of mistake won't get picked up by a compiler.

  • Plasmab (unregistered)

    I actually think he meant it as a comment marker... but missed the //

    I've come across coding standards that want you to mark the end of a loop.

  • TSK (unregistered) in reply to DaveK

    DO WHILE (condA) { (pseudocode) } REPEAT WHILE (condB);

    [Rant] Ahem, you recognized the word "(pseudocode)" ? One of the correct C/C++ alternatives is the while-loop with the break condition at the end which directly follows the pseudocode in my reply. I included pseudocode to indicate how the while-while loop should look before mentioning the C/C++ solution.

    Your if-do-alternative is correct, but it has the disadvantage that condA is mentioned twice and that it needs an "and" condition (It might look too easy to overlook, but there are already posts suggesting the wrong "or" condition). Furthermore your code contains the wrong order of short-circuit evaluation because it should be (condB && condA) instead of (condA && condB). If the conditions contain statements changing the status (like ++ and --) it is possible that the loop will be errornously leaved (condB changes the status so that condA will be only fulfilled if condB is tested).

    You can find the discussion at: Knuth, Donald E. / Floyd, Robert W. Notes on Avoiding "go to" Statements Information Processing Letters 1 (1971) p. 23-31

    From: Selected Papers on Computer Languages Donald E. Knuth Chapter 23

    You see, 1971 was a time when the languages were still primitive. Goto-haters will have a hard time because Knuth proved that regular expression conditions cannot be coded by linear alternatives and loops; the only possible solution are trivial subroutines just to avoid gotos.

  • Helm (unregistered) in reply to strcmp
    strcmp:
    char *path = strdup(getenv("MODULE_PATH")), *ptr, *saveptr;

    for (ptr = path; ptr = strtok_r(ptr, ":", &saveptr); ptr = NULL)

    RegisterPath(CreateSymbol(ptr));

    free(path);

    Wow, at least one guy actually knows how to code this "the right way(tm)", i'm relieved. And he obviously meant ptr != NULL.

Leave a comment on “While-While”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article