[ovs-dev] [PATCH 0/7] Introduce high resolution sleep support.

Ben Pfaff blp at ovn.org
Wed Nov 8 20:31:09 UTC 2017


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


More information about the dev mailing list