"brian m. carlson" [off-list ref] writes:

quoted

 #ifdef NO_SETITIMER
-#define setitimer(which,value,ovalue)
+static inline int setitimer(int which, const struct itimerval *value, struct itimerval *newvalue) {

The rest of the patch looks fine, but do we know that these structs will
exist if NO_SETITIMER is defined?  If not, we may need to use a void *
here, which would provide us worse type checking, but would work on
platforms that lack the interval timers at all, such as NonStop.

I thought about that and also making s/FILE/void/ for flockfile()
and funlockfile() for the same reason.  Indeed my first draft used
"void *".

But because these no-op macros are designed to be used in the main
codepath WITHOUT surrounding #ifdef...#endif for readability, the
platforms that use NO_SETITIMER has to declare the variable that the
calling site of setitimer() passes as its parameters, so they must
have something called "struct itimerval".  That is why I ended up
using the real type here

For example, builtin/log.c defines

    static struct itimerval early_output_timer;

and makes an unconditional call OUTSIDE any #ifdef...#endif to
setitimer(), like so:

	early_output_timer.it_value.tv_sec = 0;
	early_output_timer.it_value.tv_usec = 500000;
	setitimer(ITIMER_REAL, &early_output_timer, NULL);

I would expect that this is the use pattern any users of these
fallback definitions in git-compat-util.h should follow; those who
do not have "struct itimerval" natively indeed are using a fallback
definition from <git-compat-util.h>.

That does kind of defeat the purpose of this patch, but I still think
it's a win, since we end up with some type checking, even if it's not
perfect, and almost every platform provides setitimer, so any errors
will be caught quickly.

Yes, even if we loosen the type to "void *", it does catch certain
errors.  One thing I wrote in the log message is that moving to
"static inline" allows us to catch not just type mismatches but also
missing variables (i.e. the original code used a variable that has
been renamed, and the instances of the variable used as parameters
to these no-op macros were left unmodified).  That's not a type
mismatch but missing identifier.  The motivating example was quite
similar; it was a field renamed but left unadjusted.

Thanks.