Here are two little gems that I found recently, in a very popular piece of open-source software. See if you can figure out what’s wrong with each before reading on.

    /* d might support PROTO_XXX, PROTO_YYY, both, or neither */
    if (d->proto & PROTO_XXX) {
        send_xxx_request(d);
    }
    if (d->proto & PROTO_YYY) {
        send_yyy_request(d);
    }
    while (!got_reply && !time_expired) {
        sleep();
    }

Note that I very explicitly said at the top that d might support neither protocol. What happens then? We sleep waiting for a reply that could never possibly come to us because we never sent out either kind of request. I thought this was pretty bad, until I saw the next snippet right next door.

    /* Their label name, not mine. */
    try_try_again:
        retries = 2;
        if (try_to_do_something() == OK) {
            return SUCCESS;
        }
        if (--retries) {
            goto try_try_again;
        }
        return FAILURE;

Will this ever actually return FAILURE? No. Coding the functional equivalent of “while (2 – 1)” is bad enough, but doing it with a goto is just adding insult to injury.

The outcome of these two bugs is that something which should have failed almost immediately and moved on got stuck in an endless loop instead. The only excuse I can make for its author(s) is that it’s relatively unlikely that people will want something to fail, or configure something in a way that would fail. In this case, though, the goal was to use a single configuration in two places, even though it would only work “normally” in one place and fall back to an alternate method in the other, because maintaining two separate configurations would have been worse than having a slight delay in one.