On Tue, 9 Jul 2013, Andriy Gapon wrote:
Log: should_yield: protect from td_swvoltick being uninitialized or too staleThe distance between ticks and td_swvoltick should be calculated as an unsigned number. Previously we could end up comparing a negative number with hogticks in which case should_yield() would give incorrect answer. We should probably ensure that td_swvoltick is properly initialized. Sponsored by: HybridCluster MFC after: 5 days Modified: head/sys/kern/kern_synch.c Modified: head/sys/kern/kern_synch.c ============================================================================== --- head/sys/kern/kern_synch.c Tue Jul 9 08:59:39 2013 (r253076) +++ head/sys/kern/kern_synch.c Tue Jul 9 09:01:44 2013 (r253077) @@ -581,7 +581,7 @@ int should_yield(void) { - return (ticks - curthread->td_swvoltick >= hogticks); + return ((unsigned int)(ticks - curthread->td_swvoltick) >= hogticks); }
Hrmph. Perhaps it should be calculated as an unsigned number, but this calculates it as a signed number, with undefined behaviour if overflow occurs, and then bogusly casts the signed number to unsigned. It also has a style bug in the cast -- "unsigned int" is verbose, and even "unsigned" is too long, so it is spelled "u_int" in the kernel. Next, after the cast the operand types are mismatched, and compilers could reasonably warn about the possible sign extension bugs from this. Compilers do warn about the mismatch for "i < size_t(foo)" in loops. This warning is so annoying and it is not normally produced when both the operands are variables. Many "fixes" for this warning make sign extension bugs worse by casting changing the type of the variable instead of casting the size_t. Bruce _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
