Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Time-HiRes CPAN distribution.

Report information
The Basics
Id:
40311
Status:
resolved
Priority:
Low/Low
Queue:

People
Owner:
Nobody in particular
Requestors:
dvornik+bit [...] gmail.com
Cc:
AdminCc:

BugTracker
Severity:
Important
Broken in:
1.9715
Fixed in:
(no value)

Attachments


Subject: bad implementation of hrt_usleep with TIME_HIRES_NANOSLEEP
I was browsing the source code for HiRes.xs in Time-HiRes-1.9715, and I noticed something very broken. One of the implementations for usleep/hrt_usleep was setting its timespec structure as follows: ts1.tv_sec = usec * 1000; /* Ignoring wraparound. */ ts1.tv_nsec = 0; Of course, this is backwards; 1us is not 1000s, it's 1000ns. =) I assume that the intended code was ts1.tv_sec = 0; ts1.tv_nsec = usec * 1000; /* Ignoring wraparound. */ although it would be even better *not* to ignore wraparound: ts1.tv_sec = (usec / IV_1e6); ts1.tv_nsec = (usec % IV_1e6) * 1000; (I haven't tested the changes myself, but they ought to work). BTW, here is the current code with a bit more context: -------------------------------------------------- #if !defined(HAS_USLEEP) && defined(TIME_HIRES_NANOSLEEP) #define HAS_USLEEP #define usleep hrt_usleep /* could conflict with ncurses for static build */ void hrt_usleep(unsigned long usec) { struct timespec ts1; ts1.tv_sec = usec * 1000; /* Ignoring wraparound. */ ts1.tv_nsec = 0; nanosleep(&ts1, NULL); } #endif /* #if !defined(HAS_USLEEP) && defined(TIME_HIRES_NANOSLEEP) */ --------------------------------------------------
Please try 1.9716 or later.
From: dvornik+bit@gmail.com
On Tue Dec 30 19:21:40 2008, JHI wrote:
Show quoted text
> Please try 1.9716 or later.
I can't really "try" it, since I found the original problem by inspection of the code. But I looked at the code again, this time in more detail, and apparently I'm an idiot. =) There is no way for the code I attempted to "fix" to ever be compiled. There are *two* blocks in the code that are conditionalized by #if !defined(HAS_USLEEP) && defined(TIME_HIRES_NANOSLEEP) and provide usleep() emulation for that case. Since the first of them defines HAS_USLEEP, the second one can never be reached. The second block, which is unreachable, is the one I had tried to fix. The first one already handled wrap-around correctly, so there is nothing to fix there. Sorry for missing this fact on the first pass. I do have two low-priority cosmetic suggestions, though: - The dead block of code (the one I tried to fix) cannot ever be compiled, so it should be removed. - The other "!defined(HAS_USLEEP) && defined(TIME_HIRES_NANOSLEEP)" case defines usleep via a function named "hrt_nanosleep", which calls nanosleep() internally, but has the call semantics of usleep(). The other cases where a replacement for usleep() is being provided all call their version hrt_usleep, regardless of how the sleep is implemented internally. I think it would make sense to rename hrt_nanosleep to hrt_usleep, but I'm not 100% sure there are no binary compatibility issues. The attached patch does both of these.
--- HiRes.xs.1_9717 2008-12-31 08:19:59.875000000 -0500 +++ HiRes.xs 2008-12-31 10:45:43.031250000 -0500 @@ -402,10 +402,10 @@ * The TIME_HIRES_NANOSLEEP is set by Makefile.PL. */ #if !defined(HAS_USLEEP) && defined(TIME_HIRES_NANOSLEEP) #define HAS_USLEEP -#define usleep hrt_nanosleep /* could conflict with ncurses for static build */ +#define usleep hrt_usleep /* could conflict with ncurses for static build */ void -hrt_nanosleep(unsigned long usec) /* This is used to emulate usleep. */ +hrt_usleep(unsigned long usec) /* This is used to emulate usleep. */ { struct timespec res; res.tv_sec = usec / IV_1E6; @@ -445,21 +445,6 @@ } #endif /* #if !defined(HAS_USLEEP) && defined(WIN32) */ -#if !defined(HAS_USLEEP) && defined(TIME_HIRES_NANOSLEEP) -#define HAS_USLEEP -#define usleep hrt_usleep /* could conflict with ncurses for static build */ - -void -hrt_usleep(unsigned long usec) -{ - struct timespec ts1; - ts1.tv_sec = (usec / IV_1e6); - ts1.tv_nsec = (usec % IV_1e6) * 1000; - nanosleep(&ts1, NULL); -} - -#endif /* #if !defined(HAS_USLEEP) && defined(TIME_HIRES_NANOSLEEP) */ - #if !defined(HAS_USLEEP) && defined(HAS_POLL) #define HAS_USLEEP #define usleep hrt_usleep /* could conflict with ncurses for static build */
Subject: Re: [rt.cpan.org #40311] bad implementation of hrt_usleep with TIME_HIRES_NANOSLEEP
Date: Wed, 31 Dec 2008 22:33:15 -0500
To: bug-Time-HiRes@rt.cpan.org
From: Jarkko Hietaniemi <jhi@iki.fi>
Show quoted text
> > The attached patch does both of these.
Behold Time::HiRes 1.9718.
From: dvornik+bit@gmail.com
On Wed Dec 31 22:33:29 2008, jhi@iki.fi wrote:
Show quoted text
> > > > The attached patch does both of these.
> > Behold Time::HiRes 1.9718.
Looks good, thank you!


This service runs on Request Tracker, is sponsored by The Perl Foundation, and maintained by Best Practical Solutions.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.