Rename the global variable ``ticks''
``ticks'' is in my opinion a really badly named global variable. But we all know that finding names is hard ;) Since its popularity seems to be really high lately, I'd suggest to rename it. I chose ``hcticks'' for "hardclock ticks". I believe this would help auditing the possible existing and/or future conditions leading to overflow. As a bonus this diff removes all the "extern hcticks;" declaration and include instead. Opinions? PS: I missed 4 conversions in my previous diff, can you find them? diff --git sys/dev/atapiscsi/atapiscsi.c sys/dev/atapiscsi/atapiscsi.c index df5c45b..be164ea 100644 --- sys/dev/atapiscsi/atapiscsi.c +++ sys/dev/atapiscsi/atapiscsi.c @@ -613,7 +613,6 @@ wdc_atapi_the_machine(struct channel_softc *chp, struct wdc_xfer *xfer, enum atapi_context ctxt) { int idx = 0; - extern int ticks; int timeout_delay = hz / 10; if (xfer->c_flags & C_POLL) { @@ -637,7 +636,7 @@ wdc_atapi_the_machine(struct channel_softc *chp, struct wdc_xfer *xfer, struct atapi_return_args retargs = ARGS_INIT; (xfer->next)(chp, xfer, - xfer->endticks && (ticks - xfer->endticks >= 0), + xfer->endticks && (hcticks - xfer->endticks >= 0), ); if (retargs.timeout != -1) @@ -646,7 +645,7 @@ wdc_atapi_the_machine(struct channel_softc *chp, struct wdc_xfer *xfer, * can be just microseconds before the tick changes. */ xfer->endticks = - max((retargs.timeout * hz) / 1000, 1) + 1 + ticks; + max((retargs.timeout * hz) / 1000, 1) + 1 + hcticks; if (xfer->next == NULL) { if (xfer->c_flags & C_POLL_MACHINE) @@ -661,7 +660,7 @@ wdc_atapi_the_machine(struct channel_softc *chp, struct wdc_xfer *xfer, if (retargs.expect_irq) { int timeout_period; chp->ch_flags |= WDCF_IRQ_WAIT; - timeout_period = xfer->endticks - ticks; + timeout_period = xfer->endticks - hcticks; if (timeout_period < 1) timeout_period = 1; timeout_add(>ch_timo, timeout_period); diff --git sys/dev/ic/ar5008.c sys/dev/ic/ar5008.c index d3ad6c9..fa1c1e9 100644 --- sys/dev/ic/ar5008.c +++ sys/dev/ic/ar5008.c @@ -2381,8 +2381,7 @@ ar5008_hw_init(struct athn_softc *sc, struct ieee80211_channel *c, ar5008_init_chains(sc); if (sc->flags & ATHN_FLAG_OLPC) { - extern int ticks; - sc->olpc_ticks = ticks; + sc->olpc_ticks = hcticks; ops->olpc_init(sc); } diff --git sys/dev/ic/athn.c sys/dev/ic/athn.c index 03460ef..df2b9b1 100644 --- sys/dev/ic/athn.c +++ sys/dev/ic/athn.c @@ -1218,7 +1218,6 @@ athn_iter_func(void *arg, struct ieee80211_node *ni) void athn_calib_to(void *arg) { - extern int ticks; struct athn_softc *sc = arg; struct athn_ops *ops = >ops; struct ieee80211com *ic = >sc_ic; @@ -1229,8 +1228,8 @@ athn_calib_to(void *arg) /* Do periodic (every 4 minutes) PA calibration. */ if (AR_SREV_9285_11_OR_LATER(sc) && !AR_SREV_9380_10_OR_LATER(sc) && - (ticks - (sc->pa_calib_ticks + 240 * hz)) >= 0) { - sc->pa_calib_ticks = ticks; + (hcticks - (sc->pa_calib_ticks + 240 * hz)) >= 0) { + sc->pa_calib_ticks = hcticks; if (AR_SREV_9271(sc)) ar9271_pa_calib(sc); else @@ -1239,8 +1238,8 @@ athn_calib_to(void *arg) /* Do periodic (every 30 seconds) temperature compensation. */ if ((sc->flags & ATHN_FLAG_OLPC) && - ticks >= sc->olpc_ticks + 30 * hz) { - sc->olpc_ticks = ticks; + hcticks >= sc->olpc_ticks + 30 * hz) { + sc->olpc_ticks = hcticks; ops->olpc_temp_compensation(sc); } @@ -1279,8 +1278,7 @@ athn_init_calib(struct athn_softc *sc, struct ieee80211_channel *c, if (!AR_SREV_9380_10_OR_LATER(sc)) { /* Do PA calibration. */ if (AR_SREV_9285_11_OR_LATER(sc)) { - extern int ticks; - sc->pa_calib_ticks = ticks; + sc->pa_calib_ticks = hcticks; if (AR_SREV_9271(sc)) ar9271_pa_calib(sc); else diff --git sys/dev/ic/bwi.c sys/dev/ic/bwi.c index d612db3..7514e29 100644 --- sys/dev/ic/bwi.c +++ sys/dev/ic/bwi.c @@ -97,8 +97,6 @@ int bwi_debug = 1; #define __unused __attribute__((__unused__)) -extern int ticks; - /* XXX end porting goop */ /* MAC */ @@ -6509,11 +6507,11 @@ bwi_led_event(struct bwi_softc *sc, int event) if (event ==
Re: Rename the global variable ``ticks''
> Date: Thu, 17 Mar 2016 21:02:03 +0100 > From: Martin Pieuchot> > ``ticks'' is in my opinion a really badly named global variable. But we > all know that finding names is hard ;) > > Since its popularity seems to be really high lately, I'd suggest to > rename it. I chose ``hcticks'' for "hardclock ticks". I believe this > would help auditing the possible existing and/or future conditions > leading to overflow. > > As a bonus this diff removes all the "extern hcticks;" declaration and > include instead. > > Opinions? I don't like this kind of reshuffling. I don't think there is anything wrong with the current name. > PS: I missed 4 conversions in my previous diff, can you find them? > > diff --git sys/dev/atapiscsi/atapiscsi.c sys/dev/atapiscsi/atapiscsi.c > index df5c45b..be164ea 100644 > --- sys/dev/atapiscsi/atapiscsi.c > +++ sys/dev/atapiscsi/atapiscsi.c > @@ -613,7 +613,6 @@ wdc_atapi_the_machine(struct channel_softc *chp, struct > wdc_xfer *xfer, > enum atapi_context ctxt) > { > int idx = 0; > - extern int ticks; > int timeout_delay = hz / 10; > > if (xfer->c_flags & C_POLL) { > @@ -637,7 +636,7 @@ wdc_atapi_the_machine(struct channel_softc *chp, struct > wdc_xfer *xfer, > struct atapi_return_args retargs = ARGS_INIT; > > (xfer->next)(chp, xfer, > - xfer->endticks && (ticks - xfer->endticks >= 0), > + xfer->endticks && (hcticks - xfer->endticks >= 0), > ); > > if (retargs.timeout != -1) > @@ -646,7 +645,7 @@ wdc_atapi_the_machine(struct channel_softc *chp, struct > wdc_xfer *xfer, >* can be just microseconds before the tick changes. >*/ > xfer->endticks = > - max((retargs.timeout * hz) / 1000, 1) + 1 + ticks; > + max((retargs.timeout * hz) / 1000, 1) + 1 + hcticks; > > if (xfer->next == NULL) { > if (xfer->c_flags & C_POLL_MACHINE) > @@ -661,7 +660,7 @@ wdc_atapi_the_machine(struct channel_softc *chp, struct > wdc_xfer *xfer, > if (retargs.expect_irq) { > int timeout_period; > chp->ch_flags |= WDCF_IRQ_WAIT; > - timeout_period = xfer->endticks - ticks; > + timeout_period = xfer->endticks - hcticks; > if (timeout_period < 1) > timeout_period = 1; > timeout_add(>ch_timo, timeout_period); > diff --git sys/dev/ic/ar5008.c sys/dev/ic/ar5008.c > index d3ad6c9..fa1c1e9 100644 > --- sys/dev/ic/ar5008.c > +++ sys/dev/ic/ar5008.c > @@ -2381,8 +2381,7 @@ ar5008_hw_init(struct athn_softc *sc, struct > ieee80211_channel *c, > ar5008_init_chains(sc); > > if (sc->flags & ATHN_FLAG_OLPC) { > - extern int ticks; > - sc->olpc_ticks = ticks; > + sc->olpc_ticks = hcticks; > ops->olpc_init(sc); > } > > diff --git sys/dev/ic/athn.c sys/dev/ic/athn.c > index 03460ef..df2b9b1 100644 > --- sys/dev/ic/athn.c > +++ sys/dev/ic/athn.c > @@ -1218,7 +1218,6 @@ athn_iter_func(void *arg, struct ieee80211_node *ni) > void > athn_calib_to(void *arg) > { > - extern int ticks; > struct athn_softc *sc = arg; > struct athn_ops *ops = >ops; > struct ieee80211com *ic = >sc_ic; > @@ -1229,8 +1228,8 @@ athn_calib_to(void *arg) > /* Do periodic (every 4 minutes) PA calibration. */ > if (AR_SREV_9285_11_OR_LATER(sc) && > !AR_SREV_9380_10_OR_LATER(sc) && > - (ticks - (sc->pa_calib_ticks + 240 * hz)) >= 0) { > - sc->pa_calib_ticks = ticks; > + (hcticks - (sc->pa_calib_ticks + 240 * hz)) >= 0) { > + sc->pa_calib_ticks = hcticks; > if (AR_SREV_9271(sc)) > ar9271_pa_calib(sc); > else > @@ -1239,8 +1238,8 @@ athn_calib_to(void *arg) > > /* Do periodic (every 30 seconds) temperature compensation. */ > if ((sc->flags & ATHN_FLAG_OLPC) && > - ticks >= sc->olpc_ticks + 30 * hz) { > - sc->olpc_ticks = ticks; > + hcticks >= sc->olpc_ticks + 30 * hz) { > + sc->olpc_ticks = hcticks; > ops->olpc_temp_compensation(sc); > } > > @@ -1279,8 +1278,7 @@ athn_init_calib(struct athn_softc *sc, struct > ieee80211_channel *c, > if (!AR_SREV_9380_10_OR_LATER(sc)) { > /* Do PA calibration. */ > if (AR_SREV_9285_11_OR_LATER(sc)) { > - extern int ticks; > - sc->pa_calib_ticks = ticks; > + sc->pa_calib_ticks = hcticks; > if (AR_SREV_9271(sc)) > ar9271_pa_calib(sc); > else > diff --git sys/dev/ic/bwi.c sys/dev/ic/bwi.c > index d612db3..7514e29
Re: Rename the global variable ``ticks''
On Thu, Mar 17, 2016 at 09:02:03PM +0100, Martin Pieuchot wrote: > ``ticks'' is in my opinion a really badly named global variable. But we > all know that finding names is hard ;) > > Since its popularity seems to be really high lately, I'd suggest to > rename it. I chose ``hcticks'' for "hardclock ticks". I believe this > would help auditing the possible existing and/or future conditions > leading to overflow. > > As a bonus this diff removes all the "extern hcticks;" declaration and > include instead. > > Opinions? > I prefer the "ticks" name, but this might be because i'm too used to it. OK, to remove all the ugly extern declarations.