RE: [PATCH 1/2] Support for setitimer() on platforms lacking it
- From
Joachim Schmitz <jojo@schmitz-digital.de>
- Date
- Sep 3, 2012, 20:05 UTC
- Message-ID
- <003601cd8a0f$6a792840$3f6b78c0$@schmitz-digital.de>
- In-Reply-To
- <7v1uijexor.fsf@alter.siamese.dyndns.org>
Show 38 quoted lines
> From: Junio C Hamano [mailto:gitster@pobox.com]
> Sent: Monday, September 03, 2012 9:03 PM
> To: Joachim Schmitz
> Cc: git@vger.kernel.org; 'Johannes Sixt'
> Subject: Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
>
> "Joachim Schmitz" <jojo@schmitz-digital.de> writes:
>
> >> > if (!value ) {
> >>
> >> Style: space before ')'?
> >
> > Will fix.
> >
> >> > errno = EFAULT;
> >> > return -1;
> >>
> >> EFAULT is good ;-)
> >
> > That's what 'man setitimer()' on Linux says to happen if invalid value is found.
> >
> >> The emulation in mingw.c 6072fc3 (Windows: Implement setitimer() and
> >> sigaction()., 2007-11-13) may want to be tightened in a similar way.
> >
>
> > Hmm, I see that there the errors are handled differently, like this:
> >
> > if (ovalue != NULL)
> > return errno = EINVAL,
> > error("setitimer param 3 != NULL not implemented");
> >
> > Should this be done in my setitimer() too? Or rather be left to the caller?
> > I tend to the later.
>
> I don't care too deeply either way. The above was not a comment
> meant for you, but was to point out the error checking when the
> newvalue is NULL---it is missing in mingw.c and I think the
> condition should be checked.Ah, OK. Guess Johannes and I misunderstood ;-)
Show 7 quoted lines
> > On top here SA_RESTART is used, which is not available in HP > > NonStop (so I have a "-DSA_RESTART=0" in COMPAT_CFLAGS). > > If you cannot re-trigger the timer, then you will see "20%" shown > after one second, silence for 4 seconds and then "done", for an > operation that takes 5 seconds. Which is not the end of the world, > though. It does not affect correctness.
That does seem to work, if I do e.g. a "git clone" on git itself (being a fairly large repository), I see it updating the % values about once per second.
> The other use of itimer in our codebase is the early-output timer, > but that also is about perceived latency, and not about correctness, > so it is possible that you do not have to support anything (i.e. not > even setting an alarm) at all.
OK, I'll go for that one-liner in git-compat-utils.h then
#ifdef NO_SETITIMER /* poor man's setitimer() */ #define setitimer(w,v,o) alarm((v)->it_value.tv_sec+((v)->it_value.tv_usec>0)) #endif
It certainly seems to work just fine for me. Could as well be #ifdef __TANDEM, I won't mind.
Bye, Jojo