Re: [PATCH 9/9] Use timer_settime for new platforms

> Jacob Keller <jacob.e.keller@intel.com> writes:
> 
> > From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
> >
> > setitimer() is an obsolescent XSI interface and may be removed in a
> > future standard. Applications should use the core POSIX timer_settime()
> > instead.
> >
> > It's important that code doesn't simply check if timer_settime is
> > available as it can give false positives. Some systems like contemporary
> > OpenBSD provides the function, but it unconditionally fails with ENOSYS
> > at runtime.
> 
> Doesn't this paragraph need tweaking?  I think you lost (which is a
> good thing) "notice that timer_settime() call failed with ENOSYS and
> switch to setitimer()", no?
> 
> > Clean up the progress reporting and change it to use timer_settime,
> > which will fall back to setitimer automatically if timer_settime is not
> > supported. (see git-compat-util.h for how it does this). If both
> > functions are not present, then git-compat-util.h provides replacements
> > which will always fail with ENOSYS.
> 
> While this paragraph may be true if patch 8b and 9 are taken
> together, isn't what it describes mostly what 8b did, not 9?
> 
> Here by 8b I mean the change to git-compat-util.h in 8; the patch
> might want to be split into two, 8a for the autoconf part whose log
> message may begin with "This function was not previously used by
> git." and 8b that adds an emulation of timer_settime() API in terms
> of setitimer() API, or the other way around.
> 
> What 9 did is only "we used to use the setitmer() API to implement
> the progress reporting; now we use timer_settime() API" (yes, it is
> thanks to the abstraction given by 8, but the "callers has to only
> know about one API, not worrying about the other API" is a merit
> attributable to 8b, not this one).
> 

> > The approach used here enables us to use a single API (timer_settime)
> > without having to worry about checking for #ifdefs or if blocks which
> > make it an unreadable nightmare. The major downside is for systems
> > without timer_settime support, they may fall back on a wrapped
> > implementation which could have subtle differences. This should be a
> > minor issue as almost all modern systems provide timer_settime support.
> 
> As this paragraph.
> 
> > Note that this change means that git should never use setitimer on its
> > own now, as the fallback implementation of timer_settime assumes that it
> > is the sole user of ITIMER_REAL, and timer_delete will reset the
> > ITIMER_REAL.
> 
> And this one.
>