On Wed, Nov 08, 2017 at 04:35:52PM +0000, Bhanuprakash Bodireddy wrote: > This patchset introduces high resolution sleep support for linux and windows. > Also time macros are introduced to replace the numbers with meaningful > names.
Thank you very much for the series. Did you test that the Windows version of the code compiles (e.g. via appveyor)? I would normally squash patch 3 (the Windows version of xnanosleep) into patch 2 (the Linux version). Also, I would normally squash the patches that just replace constants by xSEC_PER_ySEC macros into the patch that introduced those macros (if there are other changes then I would separate those). I am concerned about types. The xSEC_PER_ySEC macros all use type "long" for their constants, but in some cases the code needs to have type "long long", for example in many cases when multiplying by one of these macros. When the patches replace an LL-suffixed literal by one of the xSEC_PER_ySEC macros, this introduces a risk of overflow that was not present before. I am not certain that the xSEC_PER_ySEC macros clarify things, especially given the type issues. I don't feel strongly about it though. In the xnanosleep() implementation for Windows, I think that the two calls to CloseHandle can be consolidated into one. Thanks again! Ben _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
