Reputation: 37
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 goto
s? There must be some kind of valid reason, right?
Upvotes: 0
Views: 1677
Reputation: 31366
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?
Upvotes: 3
Reputation: 69316
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.
Upvotes: 7