On Wed, Sep 25, 2024 at 8:20 AM Shuah Khan skhan@linuxfoundation.org wrote:
On 9/24/24 17:59, John Stultz wrote:
On Tue, Sep 24, 2024 at 8:57 AM Shuah Khan skhan@linuxfoundation.org wrote:
Remove local NSEC_PER_SEC and USEC_PER_SEC defines. Pick them up from include/vdso/time64.h. This requires -I $(top_srcdir) to the timers Makefile to include the include/vdso/time64.h.
posix_timers test names the defines NSECS_PER_SEC and USECS_PER_SEC. Include the include/vdso/time64.h and change NSECS_PER_SEC and USECS_PER_SEC references to NSEC_PER_SEC and USEC_PER_SEC respectively.
Nit: You got the last bit switched there. This patch changes local NSEC_PER_SEC to the upstream defined NSECS_PER_SEC.
I think what IO have is correct. posix_timers defines them as NSECS_PER_SEC and USECS_PER_SEC and the header file doesn't have the extra S. It could use rephrasing thought to make it clear.
? But the patch is removing the local non-plural NSEC_PER_SEC usage in posix_timers?
-#define NSEC_PER_SEC 1000000000LL -#define USEC_PER_SEC 1000000 -
As the headers do have the values with the extra S.
So I'm confused (sort of my natural state :), but this is a minor nit, so apologies and just ignore me if I'm really getting it backwards here.
Is it okay to fix this when I apply the patch or would you like me to send v2?
I don't need a v2
Overall no objection from me. I've always pushed to have the tests be mostly self-contained so they can be built outside of the kernel source, but at this point the current kselftest.h dependencies means it already takes some work, so this isn't introducing an undue hardship.
Yes. At this point it would be hard to build it outside. DO you think these defines can be part of uapi?
Maybe, though they are so common I fret it would cause folks trouble with redefinition collisions.
Thanks for doing this cleanup! -john