Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 3, 2012, 19:03 UTC
- Message-ID
- <7v1uijexor.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <000d01cd89b6$d5ba6c30$812f4490$@schmitz-digital.de>
"Joachim Schmitz" <jojo@schmitz-digital.de> writes:
Show 16 quoted lines
>> > 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.
>Show 8 quoted lines
> 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.
> 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.
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.