git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Joachim Schmitz" <jojo@schmitz-digital.de>
Cc: <git@vger.kernel.org>, "'Johannes Sixt'" <j6t@kdbg.org>
Subject: Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
Date: Mon, 03 Sep 2012 12:03:16 -0700	[thread overview]
Message-ID: <7v1uijexor.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <000d01cd89b6$d5ba6c30$812f4490$@schmitz-digital.de> (Joachim Schmitz's message of "Mon, 3 Sep 2012 11:31:01 +0200")

"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.

> 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.

  parent reply	other threads:[~2012-09-03 19:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-24 10:39 [PATCH 1/2] Support for setitimer() on platforms lacking it Joachim Schmitz
2012-08-28 20:15 ` Junio C Hamano
2012-08-30 16:40   ` Joachim Schmitz
2012-08-30 17:13     ` Junio C Hamano
2012-08-30 17:22       ` Joachim Schmitz
2012-09-01  9:50       ` Joachim Schmitz
2012-09-02 20:43         ` Junio C Hamano
2012-09-03  9:31           ` Joachim Schmitz
2012-09-03 18:15             ` Johannes Sixt
2012-09-03 18:57               ` Junio C Hamano
2012-09-03 19:03             ` Junio C Hamano [this message]
2012-09-03 20:05               ` Joachim Schmitz
2012-09-04 16:58                 ` Junio C Hamano
2012-09-04 17:23                   ` Joachim Schmitz
2012-09-04 18:28                     ` Junio C Hamano
2012-09-04 18:47                       ` Junio C Hamano
2012-09-04 21:47                         ` Joachim Schmitz
2012-09-04 22:44                           ` Junio C Hamano
2012-09-05  9:59                             ` Joachim Schmitz
2012-09-04 18:48                     ` Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7v1uijexor.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=jojo@schmitz-digital.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).