Rename the global variable ``ticks''

[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?

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''

[Mark Kettenis]

> 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''

[Alexandre Ratchov]

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.