On Thu, 2014-08-28 at 12:43 -0700, Junio C Hamano wrote:
> Jonas 'Sortie' Termansen <sor...@maxsi.org> writes:
> 
> > setitimer() is an obsolescent XSI interface and may be removed in a
> > future standard. Applications should use the core POSIX timer_settime()
> > instead.
> >
> > This patch cleans up the progress reporting and changes it to try using
> > timer_settime, or if that fails, setitimer. If either function is not
> > provided by the system, then git-compat-util.h provides replacements
> > that always fail with ENOSYS.
> >
> > 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.
> >
> > This approach allows the code using timer_settime() and setitimer() to
> > be simple and readable. My first attempt used #ifdef around each use of
> > timer_settime(), this quickly turned a into unmaintainable maze of
> > preprocessor conditionals.
> >
> > Signed-off-by: Jonas 'Sortie' Termansen <sor...@maxsi.org>
> > ---
> >  builtin/log.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
> >  progress.c    | 34 +++++++++++++++++++++++++++-------
> >  2 files changed, 67 insertions(+), 14 deletions(-)
> 
> Yuck.  I didn't look at the change very carefully, but are the two
> interface so vastly different that you cannot emulate one in terms
> of the other, and use a single API at the callsites, isolating the
> knowledge of which kind of API is used to interact with the system
> timer in one place (perhaps in compat/itimer.c)?
> 
> Having to sprinkle "if (is_using_timer_settime)" around means we
> need to support two APIs at each and every callsite that wants timer
> interrupt actions.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Agreed! The biggest difference is that timer_settime() uses nanoseconds
value, which is very easy to convert microseconds into nanoseconds.

In fact, you could probably easily provide a wrapper for timer_settime
that works for settimer by simply converting it. This is a much better
approach. (though settimer "could" lose some precision since you ahve to
divide by 1000)...

The other big difference is abstracting away the timer_create aspect
which is ok, since we would always call timer create, and use the
correct timer, but we just fall back to setitimer to use the default
timer.

I'll submit a patch series which does what I think should work, and will
result in less if checks.

Regards,
Jake


Reply via email to