On Tue, Nov 28, 2017 at 10:18:30PM +0000, Bodireddy, Bhanuprakash wrote:
> Hi Ben,
> 
> >On Tue, Nov 14, 2017 at 08:42:30PM +0000, Bhanuprakash Bodireddy wrote:
> >> This commit replaces the numbers with MSEC_PER_SEC, NSEC_PER_SEC and
> >> USEC_PER_MSEC macros when dealing with timespec and timeval.
> >>
> >> This commit doesn't change functionality.
> >>
> >> Signed-off-by: Bhanuprakash Bodireddy
> >> <[email protected]>
> >
> >This still seems careless and risky to me.
> >
> >For example:
> >    msecs = secs * MSEC_PER_SEC * 1LL;
> >which expands to
> >    msecs = secs * 1000L * 1LL;
> >still risks overflow on a 32-bit system (where 1000L is 32 bits long).
> >
> >The previous version of the code didn't have that problem:
> >    msecs = secs * 1000LL;
> >
> >Maybe it would be better to just leave these as-is.
> 
> I agree with you and take back my changes w.r.t introducing the time MACROS. 
> I have posted v3 version replacing the Macro.
> On an unrelated note, can you please also review the patch here that extends 
> get_process_info(). 
>            
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/340762.html
> 
> My Keepalive patch series has dependency on high resolution timer patch and 
> above mentioned API.

OK.

Thank you for your patience with my comments on this series.  I will
review v3 now.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to