incorrect timeouts "expiry" calculation on Unix systems

[aleksey-sergey]

found this cool project today.

I decided to perform some basic tests and assigned 5 seconds timeout to serial_port... But instead of 5 seconds, read() method has blocked for ~13 seconds!

I made some investigations and found that error occurs due to an overflow in "signed long" variable (tv_nsec field of timespec_t) https://github.com/wjwwood/serial/blob/master/src/impl/unix.cc#L65

however, there is no need to subtract sec_diff from tv_nsec. this operation can be easily replaced with division by modulo.

The code to reproduce an error:

#include <string>
#include <iostream>
#include "serial/serial.h"

int main(int argc, char **argv) {
    serial::Serial my_serial("/dev/ttyFAKE", 115200, serial::Timeout::simpleTimeout(5000));
    std::string line = my_serial.readline();
    return 0;
}

[wjwwood]

Wow, thanks for finding that. I can confirm this was a bug using your example. @mikepurvis FYI (not sure if you guys are using a fork of this package or using it directly).

[wjwwood]

Actually this looks to break on the tests (timer_tests.short_intervals) on the ROS buildfarm CI:

http://build.ros.org/job/Idev__serial__ubuntu_trusty_amd64/5/consoleFull

Not sure why this wasn't caught by travis, maybe it's a race condition?

[aleksey-sergey]

it's not a race condition. just found a bug in code I wrote :( https://github.com/wjwwood/serial/blob/master/src/impl/unix.cc#L65

Line 65: expiry.tv_nsec %= static_cast<int>(1e9); should be fixed by: expiry.tv_nsec = tv_nsec % static_cast<int>(1e9);

in this case expiry.tv_nsec will get correct value and should successfully pass all tests.

[wjwwood]

Ah, you're right. I was wondering about that line, but I saw the tests passing here and figured it was just working and I was not reading right.

Do you think you can open another pr that fixes it?