The Dreaded goto

Discuss all aspects of programming here. From 8-bit through to modern architectures.
Post Reply
Coeus
Posts: 1271
Joined: Mon Jul 25, 2016 11:05 am
Contact:

The Dreaded goto

Post by Coeus » Fri Nov 02, 2018 10:54 pm

I don't usually feel the call of the goto statement but what about this function:

Code: Select all

int mmb_find(const char *name)
{
    const char *cat_ptr = mmb_cat + 16;
    const char *cat_end = mmb_cat + MMB_CAT_SIZE;
    const char *cat_nxt, *nam_ptr;
    int cat_ch, nam_ch, i;

    while (cat_ptr < cat_end) {
        cat_nxt = cat_ptr + 16;
        nam_ptr = name;
        do {
            if (cat_ptr == cat_nxt)
                goto found;
            cat_ch = *cat_ptr++;
            nam_ch = *nam_ptr++;
            if (!cat_ch && !nam_ch)
                goto found;
        } while (!((cat_ch ^ nam_ch) & 0x5f));
        cat_ptr = cat_nxt;
    }
    return -1;
found:
    i = (cat_nxt - mmb_cat) / 16 - 2;
    log_debug("mmb: found MMB SSD '%s' at %d", name, i);
    return i;
}
this could be re-written without the gotos, keeping the same logic:

Code: Select all

int mmb_find(const char *name)
{
    const char *cat_ptr = mmb_cat + 16;
    const char *cat_end = mmb_cat + MMB_CAT_SIZE;
    const char *cat_nxt, *nam_ptr;
    int cat_ch, nam_ch, i;

    while (cat_ptr < cat_end) {
        cat_nxt = cat_ptr + 16;
        nam_ptr = name;
        do {
            if (cat_ptr == cat_nxt|| (!(cat_ch = *cat_ptr++) && !(nam_ch = *nam_ptr++))) {
                i = (cat_nxt - mmb_cat) / 16 - 2;
                log_debug("mmb: found MMB SSD '%s' at %d", name, i);
                return i;
            }
        } while (!((cat_ch ^ nam_ch) & 0x5f));
        cat_ptr = cat_nxt;
    }
    return -1;
}
Clearer? If not, any other alternatives?

User avatar
flaxcottage
Posts: 3635
Joined: Thu Dec 13, 2012 8:46 pm
Location: Derbyshire
Contact:

Re: The Dreaded goto

Post by flaxcottage » Sat Nov 03, 2018 9:59 am

Best not to jump out of a loop. The second way is technically better.
- John

Image

User avatar
paulv
Posts: 3854
Joined: Tue Jan 25, 2011 6:37 pm
Location: Leicestershire
Contact:

Re: The Dreaded goto

Post by paulv » Sat Nov 03, 2018 10:06 am

The if evaluation took me a couple of reads as you have equality and assignation in the statement so on first read, it looks like you've missed a couple of = symbols out accidentally but they're actually correct as they are.

For clarity, it may be worth doing the assignation before the if rather than inline to avoid confusion.

EDIT: but that then means the code changes its behaviour so I see why you've done it inline. Just makes for the need for a more attentive read through the code.

Paul
Last edited by paulv on Sat Nov 03, 2018 10:08 am, edited 1 time in total.

Phlamethrower
Posts: 103
Joined: Fri Nov 24, 2017 1:35 pm
Contact:

Re: The Dreaded goto

Post by Phlamethrower » Sat Nov 03, 2018 1:15 pm

When goto's become ugly/confusing I tend to try and restructure the code into a subroutine, so that the goto's become return's:

Code: Select all

static inline int cat_name_cmp(const char *nam_ptr, const char *cat_ptr)
{
    const char *cat_nxt = cat_ptr + 16;
    do {
        char cat_ch = *cat_ptr++;
        char nam_ch = *nam_ptr++;
        if (!cat_ch && !nam_ch)
            break;
        if ((cat_ch ^ nam_ch) & 0x5f)
            return -1;
    } while (cat_nxt != cat_ptr);
    return (cat_nxt - mmb_cat) / 16 - 2;
}

int mmb_find(const char *name)
{
    const char *cat_ptr = mmb_cat + 16;
    const char *cat_end = mmb_cat + MMB_CAT_SIZE;
    int i;

    do {
        i = cat_name_cmp(name, cat_ptr);
        cat_ptr += 16;
    } while ((i == -1) && (cat_ptr < cat_end));
    if (i != -1)
    {
        log_debug("mmb: found MMB SSD '%s' at %d", name, i);
    }
    return i;
}
If the compiler's any good it should be able to inline the function, avoiding any inefficiencies that an actual function call would introduce. Of course if you're using a vintage compiler then it's not likely to be that good at optimisation, so if you're concerned about code size/speed then an ugly/confusing function may be the best option.

Diminished
Posts: 154
Joined: Fri Dec 08, 2017 9:47 pm
Contact:

Re: The Dreaded goto

Post by Diminished » Sat Nov 03, 2018 3:42 pm

I've sometimes resorted to "do { } while (0);" to avoid a goto (in which a break statement will jump you to the end of the block).

It's messy if you want to jump out of multiple blocks, though. The goto is probably clearer.
Last edited by Diminished on Sat Nov 03, 2018 3:46 pm, edited 2 times in total.

Post Reply