From bbdda93be2f0bfefe91466b398fe42700a6ea284 Mon Sep 17 00:00:00 2001 From: riastradh Date: Wed, 16 Jun 2021 00:21:17 +0000 Subject: if_attach and if_initialize cannot fail, don't test return value These were originally made failable back in 2017 when if_initialize allocated a softint in every interface for link state changes, so that it could fail gracefully instead of panicking: https://mail-index.NetBSD.org/source-changes/2017/10/23/msg089053.html However, this spawned many seldom- or never-tested error branches, which are risky to have around. And that softint in every interface has since been replaced by a single global workqueue, because link state changes require thread context but not low latency or high throughput: https://mail-index.NetBSD.org/source-changes/2020/02/06/msg113759.html So there is no longer any reason for if_initialize to fail. (The subroutine if_stats_init can't fail because percpu_alloc can't fail either.) There is a snag: the softint_establish in if_percpuq_create could fail, potentially leading to bad consequences later on trying to use the softint. This change doesn't introduce any new bugs because of the snag -- if_percpuq_attach was already broken. However, the snag can be better addressed without spawning error branches, either by using a single softint or making softints less scarce. (Separate commit will change the signatures of if_attach and if_initialize to return void, scheduled to ride whatever is the next convenient kernel bump.) Patch and testing on amd64 and evbmips64-eb by maya@; commit message soliloquy, and compile-testing on evbppc/i386/earmv7hf, by me. --- sys/dev/pcmcia/if_malo_pcmcia.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) (limited to 'sys/dev/pcmcia') diff --git a/sys/dev/pcmcia/if_malo_pcmcia.c b/sys/dev/pcmcia/if_malo_pcmcia.c index d79fab78297..872cf4b05b5 100644 --- a/sys/dev/pcmcia/if_malo_pcmcia.c +++ b/sys/dev/pcmcia/if_malo_pcmcia.c @@ -1,4 +1,4 @@ -/* $NetBSD: if_malo_pcmcia.c,v 1.25 2020/01/29 13:54:41 thorpej Exp $ */ +/* $NetBSD: if_malo_pcmcia.c,v 1.26 2021/06/16 00:21:19 riastradh Exp $ */ /* $OpenBSD: if_malo.c,v 1.65 2009/03/29 21:53:53 sthen Exp $ */ /* @@ -18,7 +18,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: if_malo_pcmcia.c,v 1.25 2020/01/29 13:54:41 thorpej Exp $"); +__KERNEL_RCSID(0, "$NetBSD: if_malo_pcmcia.c,v 1.26 2021/06/16 00:21:19 riastradh Exp $"); #ifdef _MODULE #include @@ -307,7 +307,7 @@ cmalo_attach(void *arg) struct malo_softc *sc = arg; struct ieee80211com *ic = &sc->sc_ic; struct ifnet *ifp = &sc->sc_if; - int i, rv; + int i; /* disable interrupts */ cmalo_intr_mask(sc, 0); @@ -318,7 +318,7 @@ cmalo_attach(void *arg) cmalo_fw_load_main(sc) != 0) { /* free firmware */ cmalo_fw_free(sc); - goto fail_1; + goto fail; } sc->sc_flags |= MALO_FW_LOADED; @@ -368,11 +368,7 @@ cmalo_attach(void *arg) } /* attach interface */ - rv = if_initialize(ifp); - if (rv != 0) { - aprint_error_dev(sc->sc_dev, "if_initialize failed(%d)\n", rv); - goto fail_2; - } + if_initialize(ifp); ieee80211_ifattach(ic); /* Use common softint-based if_input */ ifp->if_percpuq = if_percpuq_create(ifp); @@ -393,12 +389,7 @@ cmalo_attach(void *arg) return; -fail_2: - cv_destroy(&sc->sc_cv); - mutex_destroy(&sc->sc_mtx); - free(sc->sc_cmd, M_DEVBUF); - free(sc->sc_data, M_DEVBUF); -fail_1: +fail: cmalo_fw_free(sc); } -- cgit