Messages in this thread Patch in this message | | | Date | Fri, 26 Jun 2009 22:33:54 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/2] do_coredump: factor out put_cred() calls |
| |
On 06/26, Roland McGrath wrote: > > > if (!binfmt || !binfmt->core_dump) > > - goto fail; > > + goto out; > [...] > > +out: > > return; > > } > > If the label does nothing, just use "return" instead of goto.
But this is what the current code does, I just renamed "fail" to "out" to make sure I didn't miss a goto.
And in fact I am not sure return is better, "goto out" looks more consistent to me.
But OK, please find the updated patch below.
-------------------------------------------------------------------------- [PATCH 1/2] do_coredump: factor out put_cred() calls
Given that do_coredump() calls put_cred() on exit path, it is a bit ugly to do put_cred() + "goto fail" twice, just add the new "fail_creds" label.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
--- WAIT/fs/exec.c~CD_1_PUT_CRED 2009-06-15 12:48:28.000000000 +0200 +++ WAIT/fs/exec.c 2009-06-26 22:30:28.000000000 +0200 @@ -1732,13 +1732,11 @@ void do_coredump(long signr, int exit_co binfmt = current->binfmt; if (!binfmt || !binfmt->core_dump) - goto fail; + return; cred = prepare_creds(); - if (!cred) { - retval = -ENOMEM; - goto fail; - } + if (!cred) + return; down_write(&mm->mmap_sem); /* @@ -1746,8 +1744,7 @@ void do_coredump(long signr, int exit_co */ if (mm->core_state || !get_dumpable(mm)) { up_write(&mm->mmap_sem); - put_cred(cred); - goto fail; + goto fail_creds; } /* @@ -1761,10 +1758,8 @@ void do_coredump(long signr, int exit_co } retval = coredump_wait(exit_code, &core_state); - if (retval < 0) { - put_cred(cred); - goto fail; - } + if (retval < 0) + goto fail_creds; old_cred = override_creds(cred); @@ -1862,9 +1857,8 @@ fail_unlock: if (helper_argv) argv_free(helper_argv); + coredump_finish(mm); revert_creds(old_cred); +fail_creds: put_cred(cred); - coredump_finish(mm); -fail: - return; }
| |