correct way to pass callbacks
Giorgos Keramidas
keramida at freebsd.org
Sun Feb 25 18:30:56 UTC 2007
On 2007-02-25 21:37, Andrew Thompson <thompsa at freebsd.org> 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;
More information about the freebsd-current
mailing list