time_t when used as timedelta

2012-10-09 Thread Erik Cederstrand
Hi list,

I'm looking at this possible divide-by zero in dhclient: 
http://scan.freebsd.your.org/freebsd-head/WORLD/2012-10-07-amd64/report-nBhqE2.html.gz#EndPath

In this specific case, it's obvious from the intention of the code that 
ip-client-interval is always 0, but it's not obvious to me in the code. I 
could add an assert before the possible divide-by-zero:

assert(ip-client-interval  0);

But looking at the code, I'm not sure it's very elegant. ip-client-interval 
is defined as time_t (see src/sbin/dhclient/dhcpd.h), which is a signed integer 
type, if I'm correct. However, some time_t members of struct client_state and 
struct client_config (see said header file) are assumed in the code to be 
positive and possibly non-null. Instead of plastering the code with asserts, is 
there something like an utime_t type? Or are there better ways to enforce the 
invariant?

Thanks,
Erik
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: time_t when used as timedelta

2012-10-09 Thread Ian Lepore
On Tue, 2012-10-09 at 17:35 +0200, Erik Cederstrand wrote:
 Hi list,
 
 I'm looking at this possible divide-by zero in dhclient: 
 http://scan.freebsd.your.org/freebsd-head/WORLD/2012-10-07-amd64/report-nBhqE2.html.gz#EndPath
 
 In this specific case, it's obvious from the intention of the code that 
 ip-client-interval is always 0, but it's not obvious to me in the code. I 
 could add an assert before the possible divide-by-zero:
 
 assert(ip-client-interval  0);
 
 But looking at the code, I'm not sure it's very elegant. ip-client-interval 
 is defined as time_t (see src/sbin/dhclient/dhcpd.h), which is a signed 
 integer type, if I'm correct. However, some time_t members of struct 
 client_state and struct client_config (see said header file) are assumed in 
 the code to be positive and possibly non-null. Instead of plastering the code 
 with asserts, is there something like an utime_t type? Or are there better 
 ways to enforce the invariant?
 

It looks to me like the place where enforcement is really needed is in
parse_lease_time() which should ensure at the very least that negative
values never get through, and in some cases that zeroes don't sneak in
from config files.  If it were ensured that
ip-client-config-backoff_cutoff could never be less than 1 (and it
appears any value less than 1 would be insane), then the division by
zero case could never happen.  However, at least one of the config
statements handled by parse_lease_time() allows a value of zero.

Since nothing seems to ensure that backoff_cutoff is non-zero, it seems
like a potential source of div-by-zero errors too, in that same
function.

-- Ian


___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org