C goto vs Zig defer / errdefer / break

To be honest, I personally consider goto approach to cleanup code
to be very nice, simple and readable. But this is the only case
where explicit goto is really needed.

Zig’s way to do cleanup in a function requires a bit more thinking,
whereas in C I do it automatically.

Once upon a time I’ve written a file compressor

1 Like

I guess in a sense you are right that goto cleanup is probably more subjective, I personally don’t like it I find it very messy but I guess it’s not the worst, I just find that having an idiom that tells you “ok if you get an error don’t worry we will execute your cleanup code” is better than to keep track of 10 different label.

ten?!? :slight_smile: you’re kidding…
One label is more than enough:

bool do_smth(... result *r) {

    int fd1 = -1;
    int fd2 = -1;
    void *m1 = NULL;
    void *m2 = NULL;

    fd1 = open(...);
    if (-1 == fd1) {
        ...
        goto __failure;
    }

    fd2 = open(...);
    if (-1 == fd2) {
        ...
        goto __failure;
    }

    m1 = malloc(...);
    if (NULL == m1) {
        ...
        goto __failure;
    }

    m2 = malloc(...);
    if (NULL == m2) {
        ...
        goto __failure;
    }

    r->fd1 = fd1;
    r->fd2 = fd2;
    r->m1 = m1;
    r->m2 = m2;
    return true;

  __failure:
    if (-1 != fd1)
        close(fd1);
    if (-1 != fd2)
        close(fd2);
    if (NULL != m1)
        free(m1);
    if (NULL != m2)
        free(m2);
    return false;
}

I’ve never seen more clear cleanup pattern than this one.

This is more what I had in mind, it always look nice in snippets not really in the real world:

int ida_alloc_range(struct ida *ida, unsigned int min, unsigned int max,
			gfp_t gfp)
{
	XA_STATE(xas, &ida->xa, min / IDA_BITMAP_BITS);
	unsigned bit = min % IDA_BITMAP_BITS;
	unsigned long flags;
	struct ida_bitmap *bitmap, *alloc = NULL;

	if ((int)min < 0)
		return -ENOSPC;

	if ((int)max < 0)
		max = INT_MAX;

retry:
	xas_lock_irqsave(&xas, flags);
next:
	bitmap = xas_find_marked(&xas, max / IDA_BITMAP_BITS, XA_FREE_MARK);
	if (xas.xa_index > min / IDA_BITMAP_BITS)
		bit = 0;
	if (xas.xa_index * IDA_BITMAP_BITS + bit > max)
		goto nospc;

	if (xa_is_value(bitmap)) {
		unsigned long tmp = xa_to_value(bitmap);

		if (bit < BITS_PER_XA_VALUE) {
			bit = find_next_zero_bit(&tmp, BITS_PER_XA_VALUE, bit);
			if (xas.xa_index * IDA_BITMAP_BITS + bit > max)
				goto nospc;
			if (bit < BITS_PER_XA_VALUE) {
				tmp |= 1UL << bit;
				xas_store(&xas, xa_mk_value(tmp));
				goto out;
			}
		}
		bitmap = alloc;
		if (!bitmap)
			bitmap = kzalloc(sizeof(*bitmap), GFP_NOWAIT);
		if (!bitmap)
			goto alloc;
		bitmap->bitmap[0] = tmp;
		xas_store(&xas, bitmap);
		if (xas_error(&xas)) {
			bitmap->bitmap[0] = 0;
			goto out;
		}
	}

	if (bitmap) {
		bit = find_next_zero_bit(bitmap->bitmap, IDA_BITMAP_BITS, bit);
		if (xas.xa_index * IDA_BITMAP_BITS + bit > max)
			goto nospc;
		if (bit == IDA_BITMAP_BITS)
			goto next;

		__set_bit(bit, bitmap->bitmap);
		if (bitmap_full(bitmap->bitmap, IDA_BITMAP_BITS))
			xas_clear_mark(&xas, XA_FREE_MARK);
	} else {
		if (bit < BITS_PER_XA_VALUE) {
			bitmap = xa_mk_value(1UL << bit);
		} else {
			bitmap = alloc;
			if (!bitmap)
				bitmap = kzalloc(sizeof(*bitmap), GFP_NOWAIT);
			if (!bitmap)
				goto alloc;
			__set_bit(bit, bitmap->bitmap);
		}
		xas_store(&xas, bitmap);
	}
out:
	xas_unlock_irqrestore(&xas, flags);
	if (xas_nomem(&xas, gfp)) {
		xas.xa_index = min / IDA_BITMAP_BITS;
		bit = min % IDA_BITMAP_BITS;
		goto retry;
	}
	if (bitmap != alloc)
		kfree(alloc);
	if (xas_error(&xas))
		return xas_error(&xas);
	return xas.xa_index * IDA_BITMAP_BITS + bit;
alloc:
	xas_unlock_irqrestore(&xas, flags);
	alloc = kzalloc(sizeof(*bitmap), gfp);
	if (!alloc)
		return -ENOMEM;
	xas_set(&xas, min / IDA_BITMAP_BITS);
	bit = min % IDA_BITMAP_BITS;
	goto retry;
nospc:
	xas_unlock_irqrestore(&xas, flags);
	kfree(alloc);
	return -ENOSPC;
}
EXPORT_SYMBOL(ida_alloc_range);

(btw that’s from the linux kernel, and sure this is not really the best example of it, but it’s not uncommon to see that in C)

1 Like

Very loosely something like this:
“And these hypocrites are prohibiting me from picking my nose”

1 Like

“whereas in C I do it automatically”

This comes off as more of an experience thing than an actual good language feature. Obviously people who have years of experience in C are going to gravitate towards C ways of doing things.

Most newbie C programmers absolutely abuse goto. I feel like defer/errdefer is easier to grasp and can be mentally carried over from higher level languages like Go and D.

Zig was very natural when I came from Go, C on the other hand is a different world.

2 Likes

This is really awful.
And as I said before the only natural use of goto is managing cleanup.
That goto is not about cleanup.

1 Like

That’s true, and to be fair goto cleanup is not the worst, but I think it is still very error prone, in the sense that, you have to make the goto statement, so you better not forget to close a ressource. Whereas errdefer or defer, they take care of that for you, and if you forget something you can get warnings from the compiler, which you don’t get in C if you forgot a statement in your goto label.

Yes, that’s true.

I disagree. Once you’ve seen goto based cleanup pattern you grasp it immediately, no explanations is needed at all.
defer/errdefer, on the contrary, require thorough explanations because they are implicit jumps akin try/catch/finally.

Moreover, cleanup operations should be run in reverse order of resource allocations. defer/errdefer do it automatically. In C you have to manually maintain correct order.

Just undo everything that failed, in any order.

__in_case_of_failure:
    if (NULL != m) {
        if (NULL != m->a) free(m->a);
        if (-1 != m->fd) {
            int res = 0;
            res = close(m->fd);
            if (-1 == res) {
                printf (
                    "%s: oooops, something went really wrong - close(%d)\n",
                    __func__, m->fd
                );
            }
       }
       free(m);
    }

For whatever it is worth, I use this pattern in C a lot:

int foo(const char* name) {
  int rc = 0;
  FILE* fp = 0;
  char* buf = 0;
  do {
    fp = fopen("name", "r");
    if (!fp) break;
    buf = malloc(fileSize(fp));
    if (!buf) break;
    // do stuff ...
  } while (0);
  if (fp) fclose(fp);
  if (buf) free(buf);
  return rc;
}

It really is a goto failure in disguise, but I like the linearity. If there is anything inside the do ... while(0) which is a bit more complex, I will usually move it to its own function which also follows this pattern.

1 Like

rc is always zero in this example.

To be in context of the joke… that sentence is used to express an utmost crux
when someone is seeing some unusual things being done by others.
I had wrenched it out of the anecdote, in which a little boy, after spying his
parents in the bedroom, fairly noted not out loud: “… and these people suppress me picking my nose ?!..”

:upside_down_face:

1 Like

In an imaginary Ziggified version of C, 47 lines become 21 and in my opinion easier to read:

!void do_smth(... result *r) {
    r->fd1 = try open(...);
    errdefer {
	    ...
	    close(r->fd1);
    }

    r->fd2 = try open(...);
    errdefer {
	    ...
	    close(r->fd2);
    }

    r->m1 = try malloc(...);
    errdefer {
	    ...
	    free(r->m1);
    }

    r->m2 = try malloc(...);
}

Not that the C code isn’t clear, it sure is, but Zig is still clear while being less error prone (by not having to jump back and forth from top to bottom to check if everything is handled) and more concise IMO. I know lines of code isn’t a crucial measure of code quality, but I do recall reading somewhere a statistic about how the number of bugs in a project rises along with the number of lines of code.

7 Likes

That’s one of the patterns that was recommended when goto was removed from Zig.

1 Like

It’s not really. Zig essentially has goto via labeled breaks

they (labeled breaks) looks very weird.

A quotation from python (sic!) manifest:

Explicit is better than implicit

I like continue, break and goto when they are explicit.
In Zig we have explicit continue and (sometimes ugly/strange/weird) break
but we do not have goto. Why so? Even D language has it.

In my C example all assignments like r->smth = smth;
are located in one solid piece of code,
whereas in that ziggified version of C they are scattered all around.