Use of goto in Linux kernel code makes no sense

Question

I was going through the Linux source code and here I stumbled over this function:

static int check_free_space(struct bsd_acct_struct *acct)
{
    struct kstatfs sbuf;

    if (time_is_after_jiffies(acct->needcheck))
        goto out;

    /* May block */
    if (vfs_statfs(&acct->file->f_path, &sbuf))
        goto out;

    if (acct->active) {
        u64 suspend = sbuf.f_blocks * SUSPEND;
        do_div(suspend, 100);
        if (sbuf.f_bavail <= suspend) {
            acct->active = 0;
            pr_info("Process accounting paused\n");
        }
    } else {
        u64 resume = sbuf.f_blocks * RESUME;
        do_div(resume, 100);
        if (sbuf.f_bavail >= resume) {
            acct->active = 1;
            pr_info("Process accounting resumed\n");
        }
    }

    acct->needcheck = jiffies + ACCT_TIMEOUT*HZ;
out:
    return acct->active;
}

I can't find much sense in Marco's use of goto, especially since it leads to a return statement. Why wasn't the function re-written like this:

static int check_free_space(struct bsd_acct_struct * acct) {
  struct kstatfs sbuf;

  if (time_is_after_jiffies(acct->needcheck) ||
    vfs_statfs( &acct->file->f_path, & sbuf)) {
    //latter may block
    return acct->active;
  }
  if (acct->active) {
    u64 suspend = sbuf.f_blocks * SUSPEND;
    do_div(suspend, 100);
    if (sbuf.f_bavail <= suspend) {
      acct->active = 0;
      pr_info("Process accounting paused\n");
    }
  } else {
    u64 resume = sbuf.f_blocks * RESUME;
    do_div(resume, 100);
    if (sbuf.f_bavail >= resume) {
      acct->active = 1;
      pr_info("Process accounting resumed\n");
    }
  }
  acct->needcheck = jiffies + ACCT_TIMEOUT * HZ;
}

I've been taught that goto could indeed be useful if used to break out of a nested loop or for memory cleanup. Neither of this is a case here, so why did Marco go for the gotos? There must be some kind of valid reason, right?

`goto` is used in many places in the kernel. `could indeed be useful if used to break out of a nested loop` - that is bad use of goto. `As for the former, why?` Because it's hard to do cleanup properly that way. If each loop allocates some memory and get's some resources, it's hard. Kernel coding guide allows for 3 levels of indendation anyway. `what alternatives` - a function. — KamilCuk, Jan 21 '20 at 12:09
It is common to use gotos for error checking to avoid too many levels of indent. Also, kernel routines want a single return point for cleanip. — stark, Jan 21 '20 at 12:09
@KamilCuk As for the former, why? As for the latter, what alternatives could there be breaking out of a nested loop? — Papa Grisha, Jan 21 '20 at 12:10
@stark but there's a single level of indent here. And by unifying it in one `if` check, we even reduce the amount of code. — Papa Grisha, Jan 21 '20 at 12:11
But you make the code uglier and more difficult to parse. It's almost always better to have a single point of entry and exit in a function. In your case, you don't return the value in the non-error case at all. The goto prevents such errors from ever happening — darnir, Jan 21 '20 at 12:13
https://www.kernel.org/doc/html/v4.10/process/coding-style.html. Section 7 — stark, Jan 21 '20 at 12:16
Your proposed replacement does not compile, it misses a return. — eckes, Jan 21 '20 at 12:49
@darnir the uglier is subejctive at best and the more difficult to parse depends more on what you're used to (goto's are jumps to somewhere else and may end up in more complex label placements). A good compiler also catches the missing return issue, so it really never happens and therefore your prevention argument is somewhat weaker than you'd want us to believe. All arguments aside, this is a style choice and the choice may differ between C and C++ code for the same developer for the cleanup code reason you mentioned. The style guide makes a choice, and it's not a bad one IMHO. — rubenvb, Jan 21 '20 at 13:08
If the kernel was written in C++, error control paths might make use of [exceptions rather than `goto`s](https://stackoverflow.com/a/18035495/545127). But it isn't. — Raedwald, Jan 21 '20 at 13:09
The use of _goto_ in the Linux kernel makes perfect sense for the reasons already explained. This question has a misleading title, it should be "I don't understand the use of goto in the Linux kernel". — Leonardo, Jul 26 '21 at 16:59

Marco Bonelli · Accepted Answer · 2020-09-24T11:24:19.667

Why wasn't the function re-written like this

The function that you just wrote is invalid. More precisely, if this block is not entered:

if (time_is_after_jiffies(acct->needcheck) ||
  vfs_statfs( &acct->file->f_path, & sbuf)) {
    vfs_statfs( &acct->file->f_path, & sbuf)) {
    //latter may block
    return acct->active;
}

Then the function will not be able to do a valid return anywhere else. The code will not even compile.

The purpose of the goto in that particular function is to execute an early return without the need of duplicating the return acct->active; line. This is a pretty common pattern which saves duplicated lines of code and sometimes also reduces the size of the resulting executable.

klutt · Answer 2 · 2023-01-19T16:36:34.327

It's the "single return" principle. Some programmers think it should be obeyed at all times.

The function would work exactly the same if you replaced goto out; with return acct->active; However, let's say you want to do this:

out:
    printf("Exiting function check_free_space()\n");
    return acct->active;

That becomes a lot easier.

Here is a question about single return: Should a function have only one return statement?

Use of goto in Linux kernel code makes no sense

2 Answers2