From owner-freebsd-current@FreeBSD.ORG Sun Feb 25 18:30:56 2007 Return-Path: X-Original-To: current@freebsd.org Delivered-To: freebsd-current@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B508816A402; Sun, 25 Feb 2007 18:30:56 +0000 (UTC) (envelope-from keramida@freebsd.org) Received: from igloo.linux.gr (igloo.linux.gr [62.1.205.36]) by mx1.freebsd.org (Postfix) with ESMTP id 3166E13C467; Sun, 25 Feb 2007 18:30:51 +0000 (UTC) (envelope-from keramida@freebsd.org) Received: from kobe.laptop (dialup179.ach.sch.gr [81.186.70.179]) (authenticated bits=128) by igloo.linux.gr (8.13.8/8.13.8/Debian-3) with ESMTP id l1PIJw5S002272 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Sun, 25 Feb 2007 20:20:12 +0200 Received: from kobe.laptop (kobe.laptop [127.0.0.1]) by kobe.laptop (8.13.8/8.13.8) with ESMTP id l1PIJnxJ004067; Sun, 25 Feb 2007 20:19:50 +0200 (EET) (envelope-from keramida@freebsd.org) Received: (from keramida@localhost) by kobe.laptop (8.13.8/8.13.8/Submit) id l1PIJnSq004066; Sun, 25 Feb 2007 20:19:49 +0200 (EET) (envelope-from keramida@freebsd.org) Date: Sun, 25 Feb 2007 20:19:49 +0200 From: Giorgos Keramidas To: Andrew Thompson , FreeBSD Current Message-ID: <20070225181948.GA3976@kobe.laptop> References: <20070225083723.GA8131@heff.fud.org.nz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20070225083723.GA8131@heff.fud.org.nz> X-Hellug-MailScanner: Found to be clean X-Hellug-MailScanner-SpamCheck: not spam, SpamAssassin (not cached, score=-4.091, required 5, autolearn=not spam, ALL_TRUSTED -1.80, AWL 0.31, BAYES_00 -2.60) X-Hellug-MailScanner-From: keramida@freebsd.org X-Spam-Status: No Cc: Subject: Re: correct way to pass callbacks X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 25 Feb 2007 18:30:56 -0000 On 2007-02-25 21:37, Andrew Thompson wrote: > Hi, > The bridgestp module needs two callbacks from the bridge when it > attaches which so far I have just passed on with the function call. > > bstp_attach(&sc->sc_stp, bridge_state_change, bridge_rtable_expire); > > I have always felt this was rather ugly so have attached a patch to put > them both in a struct, is this the right way to do it? This is one of the good ways to do it, AFAIK. It's similar to the way vnode operations are 'abstracted' away in a vop_vector struct. Converting two functino arguments to a set of callbacks inside a struct is also going to be more extensible in the future, when new callbacks of 'bridge ops' are required. Then we can add something like: struct bridge_ops_vector { .bstatechange = bridge_state_change, .brtexpire = bridge_rtable_expire, ... }; I think I like this :) > Index: bridgestp.c > =================================================================== > RCS file: /home/ncvs/src/sys/net/bridgestp.c,v > retrieving revision 1.34 > diff -u -p -r1.34 bridgestp.c > --- bridgestp.c 18 Jan 2007 07:13:01 -0000 1.34 > +++ bridgestp.c 23 Feb 2007 22:27:08 -0000 > @@ -2087,8 +2087,7 @@ DECLARE_MODULE(bridgestp, bstp_mod, SI_S > MODULE_VERSION(bridgestp, 1); > > void > -bstp_attach(struct bstp_state *bs, bstp_state_cb_t state_callback, > - bstp_rtage_cb_t rtage_callback) > +bstp_attach(struct bstp_state *bs, struct bstp_cb *cb) > { > BSTP_LOCK_INIT(bs); > callout_init_mtx(&bs->bs_bstpcallout, &bs->bs_mtx, 0); > @@ -2102,8 +2101,8 @@ bstp_attach(struct bstp_state *bs, bstp_ > bs->bs_migration_delay = BSTP_DEFAULT_MIGRATE_DELAY; > bs->bs_txholdcount = BSTP_DEFAULT_HOLD_COUNT; > bs->bs_protover = BSTP_PROTO_RSTP; > - bs->bs_state_cb = state_callback; > - bs->bs_rtage_cb = rtage_callback; > + bs->bs_state_cb = cb->bcb_state; > + bs->bs_rtage_cb = cb->bcb_rtage; > > getmicrotime(&bs->bs_last_tc_time); > > Index: bridgestp.h > =================================================================== > RCS file: /home/ncvs/src/sys/net/bridgestp.h,v > retrieving revision 1.12 > diff -u -p -r1.12 bridgestp.h > --- bridgestp.h 11 Dec 2006 23:46:40 -0000 1.12 > +++ bridgestp.h 23 Feb 2007 22:25:27 -0000 > @@ -186,6 +186,11 @@ > typedef void (*bstp_state_cb_t)(struct ifnet *, int); > typedef void (*bstp_rtage_cb_t)(struct ifnet *, int); > > +struct bstp_cb { > + bstp_state_cb_t bcb_state; > + bstp_rtage_cb_t bcb_rtage; > +}; > + > /* > * Because BPDU's do not make nicely aligned structures, two different > * declarations are used: bstp_?bpdu (wire representation, packed) and > @@ -365,7 +370,7 @@ extern const uint8_t bstp_etheraddr[]; > > extern void (*bstp_linkstate_p)(struct ifnet *ifp, int state); > > -void bstp_attach(struct bstp_state *, bstp_state_cb_t, bstp_rtage_cb_t); > +void bstp_attach(struct bstp_state *, struct bstp_cb *); > void bstp_detach(struct bstp_state *); > void bstp_init(struct bstp_state *); > void bstp_stop(struct bstp_state *); > Index: if_bridge.c > =================================================================== > RCS file: /home/ncvs/src/sys/net/if_bridge.c,v > retrieving revision 1.92 > diff -u -p -r1.92 if_bridge.c > --- if_bridge.c 11 Dec 2006 23:46:40 -0000 1.92 > +++ if_bridge.c 23 Feb 2007 22:26:48 -0000 > @@ -528,6 +528,7 @@ bridge_clone_create(struct if_clone *ifc > { > struct bridge_softc *sc, *sc2; > struct ifnet *bifp, *ifp; > + struct bstp_cb cb; > u_char eaddr[6]; > int retry; > > @@ -583,7 +584,9 @@ bridge_clone_create(struct if_clone *ifc > mtx_unlock(&bridge_list_mtx); > } > > - bstp_attach(&sc->sc_stp, bridge_state_change, bridge_rtable_expire); > + cb.bcb_state = bridge_state_change; > + cb.bcb_rtage = bridge_rtable_expire; > + bstp_attach(&sc->sc_stp, &cb); > ether_ifattach(ifp, eaddr); > /* Now undo some of the damage... */ > ifp->if_baudrate = 0;