Re: timecounting: use C99-style initialization for all timecounter structs
Scott Cheloha writes:
> Hi,
>
> If the timecounter struct changes again in the future it will be
> easier to make the change if we are using C99-style initialization
> everywhere. In general I think C99-style initialization is easier to
> read for larger structs. The timecounter struct definitely qualifies
> as "larger". We probably should already be doing this but nobody has
> bothered yet.
>
> So I will bother. This patch changes every timecounter struct to use
> C99-style initialization. Some are already using it but most are not.
>
> Yes, I am aware that this is tedious to review. I'm sorry. I think
> suffering this now will pay off in the future.
Nah, not bad at all. Could you check if you get identical object files
by chance? It would be a nice double check.
>
> Speaking of the future: in a subsequent patch I would like to remove
> several of the the zero and NULL members, as C99 guarantees that
> omission of a member at initialization causes it to be implicitly
> zeroed. For instance, there is no reason to set .tc_user if the
> timecounter has no corresponding driver in libc. There are also no
> drivers setting the .tc_poll_pps function pointer, so we can just let
> it implicitly be NULL. And if the timecounter needs no private cookie
> we don't need to explicitly set .tc_priv to NULL. Et cetera.
>
> I suppose if people prefer it we _could_ do such changes in this
> patch. I'm leaning toward not doing that. Switching to the C99 style
> *and* dropping members will make review more difficult and increase
> the likelihood of a mistake, i.e. I will accidentally break the build
> on some platform and people will yell at me, which I want to avoid.
>
> Thoughts? Preferences? ok?
If others don't mind on some grounds, I'm fairly convinced of this being
a functional no-op and so: OK gnezdo@
>
> Index: ./arch/alpha/alpha/clock.c
> ===
> RCS file: /cvs/src/sys/arch/alpha/alpha/clock.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 clock.c
> --- ./arch/alpha/alpha/clock.c6 Jul 2020 13:33:06 - 1.24
> +++ ./arch/alpha/alpha/clock.c19 Feb 2021 02:57:55 -
> @@ -64,7 +64,14 @@ int clk_irq = 0;
>
> u_int rpcc_get_timecount(struct timecounter *);
> struct timecounter rpcc_timecounter = {
> - rpcc_get_timecount, NULL, ~0u, 0, "rpcc", 0, NULL, 0
> + .tc_get_timecount = rpcc_get_timecount,
> + .tc_poll_pps = NULL,
> + .tc_counter_mask = ~0u,
> + .tc_frequency = 0,
> + .tc_name = "rpcc",
> + .tc_quality = 0,
> + .tc_priv = NULL,
> + .tc_user = 0,
> };
>
> extern todr_chip_handle_t todr_handle;
> Index: ./arch/amd64/amd64/tsc.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 tsc.c
> --- ./arch/amd64/amd64/tsc.c 24 Dec 2020 04:20:48 - 1.22
> +++ ./arch/amd64/amd64/tsc.c 19 Feb 2021 02:57:55 -
> @@ -52,7 +52,14 @@ extern u_int32_t lapic_per_second;
> #endif
>
> struct timecounter tsc_timecounter = {
> - tsc_get_timecount, NULL, ~0u, 0, "tsc", -1000, NULL, TC_TSC
> + .tc_get_timecount = tsc_get_timecount,
> + .tc_poll_pps = NULL,
> + .tc_counter_mask = ~0u,
> + .tc_frequency = 0,
> + .tc_name = "tsc",
> + .tc_quality = -1000,
> + .tc_priv = NULL,
> + .tc_user = TC_TSC,
> };
>
> uint64_t
> Index: ./arch/amd64/isa/clock.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/isa/clock.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 clock.c
> --- ./arch/amd64/isa/clock.c 6 Jul 2020 13:33:06 - 1.34
> +++ ./arch/amd64/isa/clock.c 19 Feb 2021 02:57:55 -
> @@ -116,7 +116,14 @@ u_int i8254_get_timecount(struct timecou
> u_int i8254_simple_get_timecount(struct timecounter *tc);
>
> static struct timecounter i8254_timecounter = {
> - i8254_get_timecount, NULL, ~0u, TIMER_FREQ, "i8254", 0, NULL, 0
> + .tc_get_timecount = i8254_get_timecount,
> + .tc_poll_pps = NULL,
> + .tc_counter_mask = ~0u,
> + .tc_frequency = TIMER_FREQ,
> + .tc_name = "i8254",
> + .tc_quality = 0,
> + .tc_priv = NULL,
> + .tc_user = 0,
> };
>
> int clockintr(void *);
> Index: ./arch/armv7/omap/dmtimer.c
> ===
> RCS file: /cvs/src/sys/arch/armv7/omap/dmtimer.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 dmtimer.c
> --- ./arch/armv7/omap/dmtimer.c 19 Jan 2021 18:04:43 - 1.9
> +++ ./arch/armv7/omap/dmtimer.c 19 Feb 2021 02:57:55 -
> @@ -111,7 +111,13 @@ void dmtimer_setstatclockrate(int newhz)
> u_int dmtimer_get_timecount(struct timecounter *);
>
> static struct timecounter dmtimer_timecounter = {
> - dmtimer_get_timecount, NULL, 0x, 0, "dmtimer", 0, NULL
> + .tc_get_timecount = dmtimer_get_timecount,
> +
timecounting: use C99-style initialization for all timecounter structs
Hi,
If the timecounter struct changes again in the future it will be
easier to make the change if we are using C99-style initialization
everywhere. In general I think C99-style initialization is easier to
read for larger structs. The timecounter struct definitely qualifies
as "larger". We probably should already be doing this but nobody has
bothered yet.
So I will bother. This patch changes every timecounter struct to use
C99-style initialization. Some are already using it but most are not.
Yes, I am aware that this is tedious to review. I'm sorry. I think
suffering this now will pay off in the future.
Speaking of the future: in a subsequent patch I would like to remove
several of the the zero and NULL members, as C99 guarantees that
omission of a member at initialization causes it to be implicitly
zeroed. For instance, there is no reason to set .tc_user if the
timecounter has no corresponding driver in libc. There are also no
drivers setting the .tc_poll_pps function pointer, so we can just let
it implicitly be NULL. And if the timecounter needs no private cookie
we don't need to explicitly set .tc_priv to NULL. Et cetera.
I suppose if people prefer it we _could_ do such changes in this
patch. I'm leaning toward not doing that. Switching to the C99 style
*and* dropping members will make review more difficult and increase
the likelihood of a mistake, i.e. I will accidentally break the build
on some platform and people will yell at me, which I want to avoid.
Thoughts? Preferences? ok?
Index: ./arch/alpha/alpha/clock.c
===
RCS file: /cvs/src/sys/arch/alpha/alpha/clock.c,v
retrieving revision 1.24
diff -u -p -r1.24 clock.c
--- ./arch/alpha/alpha/clock.c 6 Jul 2020 13:33:06 - 1.24
+++ ./arch/alpha/alpha/clock.c 19 Feb 2021 02:57:55 -
@@ -64,7 +64,14 @@ int clk_irq = 0;
u_int rpcc_get_timecount(struct timecounter *);
struct timecounter rpcc_timecounter = {
- rpcc_get_timecount, NULL, ~0u, 0, "rpcc", 0, NULL, 0
+ .tc_get_timecount = rpcc_get_timecount,
+ .tc_poll_pps = NULL,
+ .tc_counter_mask = ~0u,
+ .tc_frequency = 0,
+ .tc_name = "rpcc",
+ .tc_quality = 0,
+ .tc_priv = NULL,
+ .tc_user = 0,
};
extern todr_chip_handle_t todr_handle;
Index: ./arch/amd64/amd64/tsc.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
retrieving revision 1.22
diff -u -p -r1.22 tsc.c
--- ./arch/amd64/amd64/tsc.c24 Dec 2020 04:20:48 - 1.22
+++ ./arch/amd64/amd64/tsc.c19 Feb 2021 02:57:55 -
@@ -52,7 +52,14 @@ extern u_int32_t lapic_per_second;
#endif
struct timecounter tsc_timecounter = {
- tsc_get_timecount, NULL, ~0u, 0, "tsc", -1000, NULL, TC_TSC
+ .tc_get_timecount = tsc_get_timecount,
+ .tc_poll_pps = NULL,
+ .tc_counter_mask = ~0u,
+ .tc_frequency = 0,
+ .tc_name = "tsc",
+ .tc_quality = -1000,
+ .tc_priv = NULL,
+ .tc_user = TC_TSC,
};
uint64_t
Index: ./arch/amd64/isa/clock.c
===
RCS file: /cvs/src/sys/arch/amd64/isa/clock.c,v
retrieving revision 1.34
diff -u -p -r1.34 clock.c
--- ./arch/amd64/isa/clock.c6 Jul 2020 13:33:06 - 1.34
+++ ./arch/amd64/isa/clock.c19 Feb 2021 02:57:55 -
@@ -116,7 +116,14 @@ u_int i8254_get_timecount(struct timecou
u_int i8254_simple_get_timecount(struct timecounter *tc);
static struct timecounter i8254_timecounter = {
- i8254_get_timecount, NULL, ~0u, TIMER_FREQ, "i8254", 0, NULL, 0
+ .tc_get_timecount = i8254_get_timecount,
+ .tc_poll_pps = NULL,
+ .tc_counter_mask = ~0u,
+ .tc_frequency = TIMER_FREQ,
+ .tc_name = "i8254",
+ .tc_quality = 0,
+ .tc_priv = NULL,
+ .tc_user = 0,
};
intclockintr(void *);
Index: ./arch/armv7/omap/dmtimer.c
===
RCS file: /cvs/src/sys/arch/armv7/omap/dmtimer.c,v
retrieving revision 1.9
diff -u -p -r1.9 dmtimer.c
--- ./arch/armv7/omap/dmtimer.c 19 Jan 2021 18:04:43 - 1.9
+++ ./arch/armv7/omap/dmtimer.c 19 Feb 2021 02:57:55 -
@@ -111,7 +111,13 @@ void dmtimer_setstatclockrate(int newhz)
u_int dmtimer_get_timecount(struct timecounter *);
static struct timecounter dmtimer_timecounter = {
- dmtimer_get_timecount, NULL, 0x, 0, "dmtimer", 0, NULL
+ .tc_get_timecount = dmtimer_get_timecount,
+ .tc_poll_pps = NULL,
+ .tc_counter_mask = 0x,
+ .tc_frequency = 0,
+ .tc_name = "dmtimer",
+ .tc_quality = 0,
+ .tc_priv = NULL,
};
bus_space_handle_t dmtimer_ioh0;
Index: ./arch/armv7/omap/gptimer.c
===
RCS file: /cvs/src/sys/arch/armv7/omap/gptimer.c,v
retrieving revision 1.11
diff -u -p -r1.11 gptimer.c
---
