mariusaeriksen/trickle — pull request #1 by lano1106 (2013-06-26 04:35:15+00:00)
Title: Fix select() wrapper issues

Problem #1

From man 3 select

The nfds argument specifies the range of descriptors to be tested.
The first nfds
descriptors shall be checked in each set; that is, the descriptors
from zero through
nfds-1 in the descriptor sets shall be examined.

Hence in trickle-overload.c _select(), it is wrong to decrement nfds
because:

a) the same fd could be in both read & write fdsets but is counted only
once in nfds
b) in other words nfds is maxfd value in any fdsets passed in parameters
in the following scenario, the code as is would fail

delayed socket fd = 1, fd = 2 is a local pipe not found in sdhead
tailq. nfds = 3

current code would remove fd1 from fdset and decrement nfds to 2.
Hence libc_select will not consider fd2 at all.

There is no portable way to efficiently recalculate nfds once you remove
the highest fd from the sets.

The only portable way would be to use a for loop with an initial value
equal to nfds-1 up to 0 and test each bits with FD_ISSET until it
returns true.

A more efficient and non portable way would be to test for non 0 integer
in the fdset bitmap array and only test bit by bit once you find the
first
non zero integer but IMO, this is not worth the trouble. select syscall
will scan the fdset with the implementation knowledge in the most
efficient way.

My recommendation: just do not mess with nfds.

Problem #2:

delaytv points to the first dhead element delaytv. If this element gets
freed, this will result into invalid read and write when
accessing delaytv.

Problem #3:

timersub(&inittv, &curtv, &difftv);

should be

timersub(&curtv, &inittv, &difftv);

Improvement #1:

I have removed 1 gettimeofday system call and 1 difftv computation.

Signed-off-by: Olivier Langlois