On 9 May 2011 01:10, David Miller <davem@xxxxxxxxxxxxx> wrote: > From: HÃkon LÃvdal <hlovdal@xxxxxxxxx> > Date: Mon, 9 May 2011 01:08:44 +0200 > >> On 7 May 2011 21:35, matt mooney <mfmooney@xxxxxxxxx> wrote: >>> But isn't the preferred style to have a single exit point? >> >> This is generally considered to be a bad advice, see >> http://stackoverflow.com/questions/1701686/why-should-methods-have-a-single-entry-and-exit-points/1701721#1701721 >> for instance. > > That article totally ignores the issue of locking and how hard it is > to get right without single exit points, and how unlocking in > multiple spots bloats up the code. > > Definitely don't take that article's advice when working on the > kernel. > I think we agree, but my answer was probably too short, unclear and imprecise. In the case of locking and single exit points in the kernel, they are (almost always) reached through goto/labels, and this is a fine way of handling exiting a function, e.g. void bond_3ad_state_machine_handler(struct work_struct *work) { struct bonding *bond = container_of(work, struct bonding, ad_work.work); struct port *port; struct aggregator *aggregator; read_lock(&bond->lock); if (bond->kill_timers) goto out; //check if there are any slaves if (bond->slave_cnt == 0) goto re_arm; ... re_arm: queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks); out: read_unlock(&bond->lock); } I often advocate usage of goto to achive this kind of style. What I assosiate with "writing a function as single exit style" would be something like the following (and usually littered with temporary remember-this-for-later variables). void bond_3ad_state_machine_handler(struct work_struct *work) { struct bonding *bond = container_of(work, struct bonding, ad_work.work); struct port *port; struct aggregator *aggregator; read_lock(&bond->lock); if (! bond->kill_timers) { //check if there are any slaves if (bond->slave_cnt != 0) { ... } queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks); } read_unlock(&bond->lock); } And this was what I trying to reccommend against (and which the stackoverflow question is about). So most probably my assosiasion was too implicit to make my reply useful. That was not the intention, hopefully this followup clears up a little. BR HÃkon LÃvdal -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
