vhost_dev_set_owner() is an example of why come-from labels are
bad style.
devel/drivers/vhost/vhost.c
473 /* Caller should have device mutex */
474 long vhost_dev_set_owner(struct vhost_dev *dev)
475 {
476 struct task_struct *worker;
477 int err;
478
479 /* Is there an owner already? */
480 if (vhost_dev_has_owner(dev)) {
481 err = -EBUSY;
482 goto err_mm;
What does goto err_mm do? It's actually a do-nothing goto. It would
be easier to read as a direct return. Why is it called err_mm? Because
originally the condition was:
if (dev->mm) {
err = -EBUSY;
goto err_mm;
}
We've changed the code but didn't update the label so it's slightly
confusing unless you know how vhost_dev_has_owner() is implemented.
483 }
484
485 /* No owner, become one */
486 dev->mm = get_task_mm(current);
487 worker = kthread_create(vhost_worker, dev, "vhost-%d", current->pid);
488 if (IS_ERR(worker)) {
489 err = PTR_ERR(worker);
490 goto err_worker;
491 }
492
493 dev->worker = worker;
494 wake_up_process(worker); /* avoid contributing to loadavg */
495
496 err = vhost_attach_cgroups(dev);
497 if (err)
498 goto err_cgroup;
499
500 err = vhost_dev_alloc_iovecs(dev);
501 if (err)
502 goto err_cgroup;
This name doesn't make sense because it's a come-from label which is
used twice. Some people do:
if (err)
goto err_iovecs;
503
504 return 0;
Then they add two labels here:
err_iovecs:
err_cgroup:
kthread_stop(worker);
But if you base the label name on the label location then it makes
sense. goto stop_kthread; goto err_mmput;.
505 err_cgroup:
506 kthread_stop(worker);
507 dev->worker = NULL;
508 err_worker:
509 if (dev->mm)
510 mmput(dev->mm);
511 dev->mm = NULL;
512 err_mm:
513 return err;
514 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
