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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 3, 2012, 18:57 UTC
Message-ID: <7v627vexxv.fsf@alter.siamese.dyndns.org>
In-Reply-To: <5044F3DB.9060908@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 15 quoted lines

> Am 03.09.2012 11:31, schrieb Joachim Schmitz:
>> 
>> 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.
>
> The error message is really just a reminder that the implementation is
> not complete. Writing it here has the advantage that it is much more
> accurate than a generic "invalid argument" or "operation not supported"
> error that the caller would be able to write.

Joachim quoted irrelevant (to you) part and made comments on it, but the issue I raised by Ccing you was about diagnosing NULL passed in newvalue parameter, which Joachim's code did like this:

    > int git_setitimer(int which, const struct itimerval *value,
    > 				struct itimerval *ovalue)
    > {
    > 	int ret = 0;
    >
    > 	if (!value ) {
    > 		errno = EFAULT;
    > 		return -1;

    EFAULT is good ;-)

    The emulation in mingw.c 6072fc3 (Windows: Implement setitimer() and
    sigaction()., 2007-11-13) may want to be tightened in a similar way.

but mingw.c doesn't seem to.