Re: [PATCH 1/2] Support for setitimer() on platforms lacking it

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/2] Support for setitimer() on platforms lacking it

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:37

"Joachim Schmitz" [off-list ref] writes:
quoted
quoted
	if (!value ) {
Style: space before ')'?
Will fix.
 
quoted
quoted
		errno = EFAULT;
		return -1;
EFAULT is good ;-)
That's what 'man setitimer()' on Linux says to happen if invalid value is found.
 
quoted
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.
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.

RE: [PATCH 1/2] Support for setitimer() on platforms lacking it

From: Joachim Schmitz <hidden>
Date: 2016-06-15 22:54:37

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" [off-list ref] writes:
quoted
quoted
quoted
	if (!value ) {
Style: space before ')'?
Will fix.
quoted
quoted
		errno = EFAULT;
		return -1;
EFAULT is good ;-)
That's what 'man setitimer()' on Linux says to happen if invalid value is found.
quoted
The emulation in mingw.c 6072fc3 (Windows: Implement setitimer() and
sigaction()., 2007-11-13) may want to be tightened in a similar way.
quoted
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 ;-)
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help