[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Python or Perl for a Debian maintainance project?

>>>>> "Andrew" == Andrew Suffield <asuffield@debian.org> writes:

    Andrew> It does if you do it right. I don't exactly write this sort of
    Andrew> code infrequently; it's quite easy to structure your code in a
    Andrew> manner that works neatly and effectively.

Perhaps you want to enlighten the kernel developers.  A typical kernel
function, say sys_getpgid, is implemented (in 2.4.24 kernel) like this:

asmlinkage long sys_setpgid(pid_t pid, pid_t pgid)
        struct task_struct * p;
        int err = -EINVAL;

        if (!pid)
                pid = current->pid;
        if (!pgid)
                pgid = pid;
        if (pgid < 0)
                return -EINVAL;

        /* From this point forward we keep holding onto the tasklist lock
         * so that our parent does not change from under us. -DaveM

        err = -ESRCH;
        p = find_task_by_pid(pid);
        if (!p)
                goto out;

        if (p->p_pptr == current || p->p_opptr == current) {
                err = -EPERM;
                if (p->session != current->session)
                        goto out;
                err = -EACCES;
                if (p->did_exec)
                        goto out;
        } else if (p != current)
                goto out;
        err = -EPERM;
        if (p->leader)
                goto out;
        if (pgid != pid) {
                struct task_struct * tmp;
                for_each_task (tmp) {
                        if (tmp->pgrp == pgid &&
                            tmp->session == current->session)
                                goto ok_pgid;
                goto out;

        p->pgrp = pgid;
        err = 0;
        /* All paths lead to here, thus we are safe. -DaveM */
        return err;

It is *full* of goto.  Those who are used to writing application code will
find it is all too tempted to simply "return" at the point of error, and
learn bitterly that the lock they are holding is not unlocked, leaving them
a dead kernel.  It is possible to write something like this

      if (something with error)
          return do_unlock_and_return(&tasklist_lock, err);

but it is more difficult to make sure that you unlock everything (originally
you just have to make sure that there is no "return" statement after
locking, now you have to make sure there is no "return" statement which is
not guarded by the function after locking); and it generate more code which
is mostly unused (the function is going to be inlined into the function
given its small size).  And its one more function, which means more
difficult to maintain.

For me, if "goto" is used in a disciplined way, one shouldn't be afraid to
use them, especially if it simplify your code.


Reply to: