Re: [PATCH] Incorrect value for SIGRTMAX, MIPS nonsense removed, timer_gettime fix

[George Anzinger <george@mvista.com>]

Andrew Morton wrote:
> George Anzinger <george@mvista.com> wrote:
> 
>>The attached patch does the following:
>>
>>Removes C++ comment in favor of C style.
>>
>>Removes the special treatment for MIPS SIGEV values.  We only require (and error 
>>if this fails) that the SIGEV_THREAD_ID value not share bits with the other 
>>SIGEV values.  Note that mips has yet to define this value so when they do...
>>
>>Corrects the check for the signal range to be from 1 to SIGRTMAX inclusive.
>>
>>Adds a check to verify that kmem_cache_alloc() actually returned a timer, error 
>>if not.
>>
>>Fixes a bug in timer_gettime() where the incorrect value was returned if a 
>>signal was pending on the timer OR the timer was a SIGEV_NONE timer.
> 
> 
>>-	if ((event->sigev_notify & ~SIGEV_NONE & MIPS_SIGEV) &&
>>-			event->sigev_signo &&
>>-			((unsigned) (event->sigev_signo > SIGRTMAX)))
>>+	if (((event->sigev_notify & ~SIGEV_THREAD_ID) != SIGEV_NONE) &&
>>+	    ((unsigned int) (event->sigev_signo - 1) >= SIGRTMAX))
>> 		return NULL;
> 
> 
> I was wondering if someone would try this one :( Really, this is just over
> the top.  Take pity upon your readers, and do:

I was rather thinking of educating them :)  It does produce better code...
> 
> 	if (((event->sigev_notify & ~SIGEV_THREAD_ID) != SIGEV_NONE) &&
> 		(event->sigev_signo <= 0 || event->sigev_signo > SIGRTMAX))
> 
> 
>>@@ -804,7 +826,7 @@
>> 	 * equal to jiffies, so the timer notify function is called directly.
>> 	 * We do not even queue SIGEV_NONE timers!
>> 	 */
>>-	if (!(timr->it_sigev_notify & SIGEV_NONE)) {
>>+	if (!((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE)) {
>> 		if (timr->it_timer.expires == jiffies)
>> 			timer_notify_task(timr);
>> 		else
> 
> 
> Are you sure this is correct?   If so, using != would be clearer.

Yes, if he said SIGEV_NONE we don't want to deliver a signal.  The restatement 
is OK with me.

Shall I resubmit?
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml