At least on Linux, all values the tuple returned by
os.times() are bogus. The code always divides the
return value from the times() system call by HZ, but
according to the man page times(2) they are in clock
ticks. The man page adds:

NOTES
       The number of clock ticks per second can be
obtained using
              sysconf(_SC_CLK_TCK);
       In  POSIX-1996 the symbol CLK_TCK (defined in
<time.h>) is mentioned as
       obsolescent. It is obsolete now.

       [...]

       Note that clock(3) returns values of type
clock_t that are not measured
       in clock ticks but in CLOCKS_PER_SEC.

On i386 Linux, HZ is #defined as sysconf(_SC_CLK_TCK)

/usr/include/asm-i386/param.h:7:#define HZ sysconf(_SC_CLK_TCK)

so times does the right thing.  On x86_64 HZ is defined as 100, but it 
is the same value as sysconf returns.

I could not find an authoritative statement in this regard, but it 
appears that on modern Linuxes posix_times implementation usin HZ is 
correct.


On the other hand, os.times would be more useful if it just returned a 
tuple of clock ticks.  I suggest implementing os._times returning 
integer clock ticks in posixmodule and reimplementing os.times in os.py 
in terms of sysconf and _times.

I'm attaching a test script (test_times.py) that forks a child which
runs for 5 seconds, waits for the child, then prints the time taken by
the child according to os.times().

I have a machine where os.times() reproducably reports that 8.33 seconds
have been spent, although indeed only 5 seconds pass:

======================================
# time python test_times.py
8.33333333333

real    0m5.018s
user    0m5.008s
sys     0m0.008s
======================================

I don't know which characteristics of the machine are causing this.

Well, 8.3333/5 equals 100/60. Go figure.

test_times.py produces the correct value on Linux for me, but I see the
same bogus value as Malte on OSX.

Here's three tests with different pythons on the same machine:
# ./python ../test_times.py
8.33333333333
# python ../test_times.py
8.33333333333
# python2.5 ../test_times.py
5.0

The first Python is current trunk, built just now.
The second Python is the vendor-installed (SuSE) Python 2.5.
The third Python is a Python 2.5 I installed manually from source some
time ago. Strange that it would differ from the second; it appears to be
the same revision as the second from the greeting message.

Anyway, I can reproduce the error in the trunk, which is good.
This is a 64-bit SuSE Linux machine (Xeon X5355 @ 2.66GHz).

I'm attaching a patch against trunk that fixes the problem for me
(os_times.PATCH).

This uses the sysconf values when HAVE_SYSCONF is defined, and otherwise
falls back on the old behaviour (use HZ if that is defined, 60 
otherwise).

I'm not sure if this is stylistically ok (#ifdefs inside a function,
etc.), so I'd appreciate comments.

Should I add a test case for the test suite?

Another comment: Since the fallback value of 60 was wrong in the past,
it may likely be wrong in the future. Should that fallback be removed
and replaced by a compile-time error? And is the "HZ" fallback necessary
at all? I don't know enough about Posix to know whether or not
HAVE_SYSCONF should be implied by HAVE_TIMES.

Here is a patch that adds a test case to test_posix.py. This is ignored
on Windows, as it requires fork.

There is a trade-off: If WAIT_TIME isn't large enough, small
irregularities in the process scheduler might cause this to fail. If it
is too large, the unit tests will take long for everyone.

Currently, WAIT_TIME is 0.1 seconds and the tolerance in the assertion
is about 5%, which works well for me. Not sure how this behaves on very
busy machines.

Well first I can't reproduce the bug on my machine :)
However the two patches do not produce any regression either.
I have some questions:
1. isn't 0.1 for WAIT_TIME a bit too low? 1.0 would probably be less
fragile IMHO
2. why do you fork() in test_times, rather than simply compute the
increase in the first and second return values of os.times()?

Using 1.0 would certainly be more robust. I wasn't sure if a slow-down
of "make test" by 1 second just for this one bug would be acceptable.

Regarding the fork, when I first encountered this bug, it was in the
context of measuring the runtime of child processes, so that's what I
tried to reproduce. But looking at the code, the bug should occur just
as well with the running process itself. So you're right; one could just
busy-wait for a second and then look at times()[0] and times()[1].

Note: My original unit test fails to take account that previous unit
tests may have spawned child processes. The correct behaviour is of
course to call os.times() before and after the fork and compute the
difference.

I'm not uploading a modified patch because from previous comments it
looks like a different test will be used anyway.

Malte, Antoine and I discussed this a bit on #python-dev and concluded
that the correct behaviour should be trying sysconf first, then HZ, or
raise an exception if not even HZ is available (since whichever static
value we chose would be misleading anyway).

I'm attaching a patch (modified from Malte's) which implements that.

Attaching a new test (test_posix2.PATCH) that doesn't fork and fixes the
problem with the previous test not taking previously elapsed time into
account. This supersedes test_posix.PATCH.

I left the wait time at 0.1; if we stay within the same process, this
should be large enough. A busy wait loop for 0.1 seconds should easily
get the 5% precision required by the assertAlmostEqual.

I was wrong -- 0.1 isn't enough, because os.times() typically has 0.01s
resolution, so we can easily get 0.1 vs. 0.11 which will fail the
assertion. Cranked up the WAIT_TIME to 0.3 in the attached patch
(test_posix3.PATCH). Sorry for the noise.

Sorry, but the test was still wrong because I misunderstood how
assertAlmostEqual works. Attaching a fourth (final?) test.

Malte noticed that my previous patch won't compile when HAVE_SYSCONFIG
and HZ are not defined. My bad, silly mistake.

I've attached a new version, which compiles and has been tested on all
three cases (with test_posix4.PATCH). Please, someone with privileges
remove os_times2.PATCH.

os_times3.PATCH works for me on Mac OS 10.4 and RHEL.  I have a few 
comments on the patch:

1. sysconf return type is long, not clock_t
2. If sysconf is present, but _SC_CLK_TCK is not supported, it will 
return -1.  In this case we should fall back to system HZ if available.
3. Use -1 instead of NULL as the invalid value. NULL has too strong 
connotation with pointers.
4. On systems where fixed HZ is correct calling sysconf(_SC_CLK_TCK) on 
every times call is an overkill.

I would suggest that instead of patching posixmodule.c an appropriate 
system-dependent value for HZ should be defined in configure.h.  
Unfortunately I am not familiar enough with autoconf to prepare a patch.

Alexander, regarding your comments:

1. sysconf in general returns a long because it can return all sorts of
information, but os.times() returns clock_t items, so the _SC_CLK_TCK
value must comfortably fit into a clock_t. It's preferable to cast into
a clock_t immediately rather than doing a conversion for each of the
ensuing divisions.

2. Do you have indications that such platforms exist? In that case,
indeed the patch should be adapted. Is that -1 return value documented
somewhere?

3. I agree; 0 or -1 would be better.

4. You're right about the overhead, but someone (amk?) measured it and
it's only 5% compared to the old buggy behaviour. It's still possible to
do a million calls to os.times() from Python in a second, which is
plenty fast enough. Clearly the speed could be improved, but it doesn't
appear worth the complications to me.

> 1. .. It's preferable to cast into a clock_t immediately rather than
> doing a conversion for each of the ensuing divisions.

If that's your motivation, then you should cast to double instead. 
However, I would leave it to compiler to do micro-optimizations like 
these. I am not aware of a standard that says that clock_t must be wider 
than long.  I agree that it is unlikely to produces wrong results given 
that we are realistically talking about 50-1000 range, but on some 
platforms you may see a warning.

> 2. .. Is that -1 return value documented somewhere?

Yes, see man sysconf on a sufficiently unix-like system or http://www.opengroup.org/onlinepubs/009695399/functions/sysconf.html

> 4. You're right about the overhead, but someone (amk?) measured it and
> it's only 5% compared to the old buggy behaviour. It's still possible 
to
> do a million calls to os.times() from Python in a second, which is
> plenty fast enough. Clearly the speed could be improved, but it
> doesn't appear worth the complications to me.

5% is a lot and IIRC os.times is used by some standard python profilers 
and 5% slowdown will affect people.

What I suggest is a simpler solution than your patch:

(1) Define USE_SYSTEM_HZ in config.h (will require some autoconf 
hacking).

(2) Define Py_HZ as system HZ on the systems where HZ can be trusted. 
(Some systems already define HZ as sysconf(_SC_CLK_TCK)) and fix the 
system definition appropriately where necessary.

(3) Replace HZ with Py_HZ in posixmodule.c

The advantage is that the systems where os.times is not broken will not 
be affected.

BTW, does anyone know if sysconf(_SC_CLK_TCK)) result can change during 
process lifetime?

Aha, I should read my own sources:

"The value shall not change during the lifetime of the calling process, 
[XSI]   except that sysconf(_SC_OPEN_MAX) may return different values 
before and after a call to setrlimit() which changes the RLIMIT_NOFILE 
soft limit." 
<http://www.opengroup.org/onlinepubs/009695399/functions/sysconf.html>

So we can consider making ticks_per_sec static and initializing it in 
posixinit.

FWIW, the following minimal patch fixed the bug on MacOS X and does not 
affect Linux:

===================================================================
--- Modules/posixmodule.c       (revision 61014)
+++ Modules/posixmodule.c       (working copy)
@@ -5923,7 +5923,7 @@
 
 #ifdef HAVE_TIMES
 #ifndef HZ
-#define HZ 60 /* Universal constant :-) */
+#define HZ sysconf(_SC_CLK_TCK)
 #endif /* HZ */
 
 #if defined(PYCC_VACPP) && defined(PYOS_OS2)

Hi Alexander,

> 5% is a lot and IIRC os.times is used by some standard python profilers 
> and 5% slowdown will affect people.

Profiled runs are always slower than non-profiled runs, so I'm not
convinced it is very important. You use profiling only when you need it,
not in normal production conditions.

However, fetching the value only once and then caching it is a valid
approach, so you can produce an updated patch for that if you want :-)

Alexander, speed-wise your patch is worse than the original one on
systems where HZ isn't predefined, because it calls sysconf 5 times in
each call to os.times, rather than only once per call.

If you worry about speed, the approach outlined in Antoine's last
message makes most sense to me. Doing this in the configure script
appears dangerous to me; it is well possible that the clock tick value
changes over time on the same machine (e.g. after a kernel upgrade), so
this should be determined upon process initialization, not before
compilation.

Also, I don't think that the HZ value should be preferred to the sysconf
value if both are available, as all times man pages I could check
mention sysconf as the correct way to do this, not HZ. (Some of them
state that HZ is used on "older systems".)

Finally, your patch assumes that HAVE_TIMES implies HAVE_SYSCONF; is
that guaranteed? In particular, it's not clear to me what happens on
Windows (see comment at the top of the file). I also have no idea how
any of the earlier patches behaves on Windows, unfortunately.

I suggest that you define "static int hz" and assign a value to the var
in INITFUNC().

#ifdef HZ
hz = HZ;
#elif defined(HAVE_SYSCONF)
hz = sysconf(_SC_CLK_TCK);
#else
hz = 60; /* It's 50Hz in Europe */
#endif

Tiran, that's the general approach we should follow, yes.

But the people who discussed this on #python-dev all felt a bit queasy
about the "60" fallback -- this is what caused the bug in the first
place on Guido's and my machine. (A value of 60 was assumed; 100 would
have been correct.) Having no such fallback would be preferable, unless
it's necessary.

You use Windows, right? Can you test if that fallback is necessary
there? As far as I can tell, it should not be necessary on a more
UNIX-ish system.

Never mind, on Windows a different code path is chosen.
I'm now working on a patch that computes the hz value during module
initialization. So should I keep the 60 magic value? What is the
justification?

I don't *use* Windows except for some computer games. But I'm doing some
development for Python on Windows in a VMWare box. I'm going to check HZ
for you. Give me a couple of minutes to boot the image.

How do you like this idea. Since HZ is only used in posix_times it's
fine to cache it in a local static var.

static PyObject *
posix_times(PyObject *self, PyObject *noargs)
{
	static int hz = -1;
	struct tms t;
	clock_t c;

	if (hz == -1) {
		/* Py_HZ may call sysconf(), cache the value */
		hz = Py_HZ;
	}
	errno = 0;
	...

Here is an updated patch (os_times4.PATCH) that incorporates Christian's
suggestions. The patch includes the new unit test, so test_posix?.PATCH
need not be applied separately.

I again made the unit test a bit more lenient to allow an absolute error
of 0.02 seconds, as there may be systems where the clock tick
granularity is only 1/60 seconds, and then the old tolerance of 0.015
seconds would be too low.

This patch prefers sysconf where it is available; this is what "man 2
times" asks to do. If sysconf is available but produces an error, that
error is raised. (Errors should never pass silently.) HZ is only used if
sysconf is not available. If neither sysconf nor HZ is available, a
compile-time error is raised -- in that case, HAVE_TIMES shouldn't have
been defined in the first place.

I also timed this; there is no discernible change compared to the old
behaviour.

The patch fixes the buggy behaviour on my 64-bit Linux box and makes no
difference on my 32-bit Linux box. The new unit test passes on both
machines.

Here is my take (posixmodule.diff).  When ticks per seconds cannot be 
determined, python should not provide times rather than refuse to build.  
Initialization moved to the init function.

I'd prefer a noisy compile error, since in situations where times is
available but unusable, HAVE_TIMES shouldn't have been #defined in the
first place. (That is, I'd see that as a bug in the configure script.)

But this is turning into a bikeshed discussion. I care more about the
bug being fixed than about how precisely it is fixed. For the record,
Alexander's patch fixes the bug on my Linux box, too, so I'm fine with
either patch.

If Alexander's patch ends up being applied, I suggest ripping the new
unit test from os_times4.PATCH, since the timing in test_posix4.PATCH is
fragile as mentioned above.

Either way, this looks ready to be closed to me.

> I'd prefer a noisy compile error ..

That would be fine if you could verify that none of the currently 
supported platforms will be affected. I would still feel uneasy about 
refusing to build python simply because os.times is not ported to a 
platform.

> HAVE_TIMES shouldn't have been #defined in the
> first place. (That is, I'd see that as a bug in
> the configure script.)

No, defined HAVE_TIMES only tell you that the system has 'times' 
function in the C library.  It is not intended to mean that os.times is 
implementable.

Personally, I would still prefer a one-line change that I proposed 
above.  It is obviously better than the current smiley code and if it 
happens to fix the platforms where errant behavior was observed, it is 
worth applying even if theoretically it may be wrong.

In any case, there is plenty of material here for a developer to step in 
and close the issue.

IMO, if there is no available way to compute HZ, a NotImplementedError
should be raised rather than using the 60 magic value.

>> I'd prefer a noisy compile error ..
>
> That would be fine if you could verify that none of the currently 
> supported platforms will be affected. I would still feel uneasy about 
> refusing to build python simply because os.times is not ported to a 
> platform.

Unless I'm missing something, your suggested one-line change fails to
compile in exactly the same cases -- if HAVE_TIMES is defined, but HZ
and sysconf unavailable -- but with a worse error message.

> HAVE_TIMES shouldn't have been #defined in the
> first place. (That is, I'd see that as a bug in
> the configure script.)

> No, defined HAVE_TIMES only tell you that the system has 'times' 
> function in the C library.  It is not intended to mean that os.times
> is implementable.

Sure, but if times is in the standard library, but its output is
uninterpretable, then there's something wrong going on that needs to be
fixed rather than swept under the rug.

> Personally, I would still prefer a one-line change that I proposed 
> above.  It is obviously better than the current smiley code and if it 
> happens to fix the platforms where errant behavior was observed, it is 
> worth applying even if theoretically it may be wrong.

You complained in msg62869 about the original patch that calling sysconf
on every call leads to an unacceptable slowdown. Your one-line patch
calls sysconf five times on each call when HZ is not defined.

s/standard library/system library/, of course.
Also, the original code is wrong in preferring HZ over the sysconf value.

Antoine, none of the recently proposed patches uses the 60 magic value.

Alexander's patch doesn't define times in that case (leading to an
AttributeError when trying to call os.times) while my patch complains
about the bogus environment during compilation.

I think it's better to make it a runtime error (upon invocation of
os.times()), rather than a compile-time error. But it's quite
theoretical until we find a system where the error does occur, anyway :)

> You complained in msg62869 about the original patch that calling 
sysconf
> on every call leads to an unacceptable slowdown. Your one-line patch
> calls sysconf five times on each call when HZ is not defined.

That's why I said that was a matter of personal preference.  I use 3 
different systems: i386 linux, x86_64 linux and Mac OS X and the last is 
not mission critical.  On i386 linux HZ is already defined as sysconf 
call, on x86_64 HZ is defined as 100 and it happens to be the correct 
value.  On MacOS X, HZ is not defined, and os.times exhibits the bug. 
One-line patch will make MacOS X behave the same as i386 Linux, which is 
fine for me, but I will oppose any changes that affect x86_64 linux 
performance.

Does anyone know of a platform where HZ is defined to a wrong value?  WE 
may want to undef or redefine HZ on such platforms.

> but I will oppose any changes that affect x86_64 linux 
performance.

Alexander, as I said I'm really curious about any situation where
os.times() is so performance-critical that a 5% slowdown for that
function call is unacceptable. Even when a profiler is involved,
os.times() is not the only overhead added by profiling, there is also
all the bookkeeping needed for recording various statistics... Perhaps
by profiling the profiler we would have an answer :)

Alexander, your one-line patch *does* affect performance on my 64-bit
Linux machine in a worse way than any other proposed patch by calling
sysconf five times. HZ may be defined on your machine, but it isn't on
my (Xeon) machine.

I checked man pages on four different Linuxes (32 bit and 64 bit; SuSE,
Fedora, Ubuntu; recent or six years old). All of them state that using
the sysconf value is the right thing to do. This is also stated in the
man page excerpt in Guido's original bug report.

Neither your latest patch (posixmodule.diff) not my latest patch
(os_times4.PATCH) affects performance; they both only call sysconf once
and then used a cached value.

I'm perfectly fine with your posixmodule.diff, which also meets
Antoine's criteria. I suggest we apply that patch, along with the unit
test from os_times4.PATCH, and be done with it.

>  HZ may be defined on your machine, but it isn't on my (Xeon) machine.

Are you sure?  Please note that HZ will not show up in grep -w HZ 
/usr/include because the right header file further up the tree.

On Solaris:
/usr/include/sys/param.h:#define        HZ              
((clock_t)_sysconf(_SC_CLK_TCK))

On 32-bit Linux:
/usr/include/asm-i386/param.h:#define HZ sysconf(_SC_CLK_TCK)

On 64-bit Linux:
/usr/include/asm-x86_64/param.h:#define HZ 100

Did you try to run posixmodule.c through gcc -E on your system?

I should not play devil's advocate and argue against my own patch, but 
there are two issues:

1. If a system provides non-standard HZ, is it to be preferred to 
sysconf(..)?  Are there systems with correct HZ but no sysconf?

2. Is the added complexity of #ifdef dance justified for the performance 
improvements on some platforms?  I know it looks like I am flip-flopping 
here, but I just don't want to change anything on the platforms where 
os.times is not broken and use a solution that is know to work on some 
platforms to fix the bug where it shows up.

If I remove the "#define 60" line in an unmodified posixmodule.c from
trunk, I get the following compiler error:

gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O3 -Wall
-Wstrict-prototypes  -I. -IInclude -I./Include   -DPy_BUILD_CORE  -c
./Modules/posixmodule.c -o Modules/posixmodule.o
./Modules/posixmodule.c: In function posix_times:
./Modules/posixmodule.c:5964: error: HZ undeclared (first use in this
function)

So yes, HZ isn't available there. It sure is defined *somewhere* (grep
shows a definition in /usr/include/asm-x86_64/param.h), but it isn't
anywhere Python would pick it up.

And I don't really see why it should when the man pages all argue that
using HZ for times is for "older system" (this is a man page from 2002,
no less!). Can you measure a performance difference between your patch
and the old buggy behaviour? I couldn't, on any machine, and I'd be very
surprised if it existed. There is no significant difference between
dividing by a constant and diving by a static module variable, and, as
Antoine rightly suggests, any such difference would be completely lost
in the noise considering the hefty cost of the other operations.

Regarding your two issues:
1. Yes, the sysconf value should take precedence over HZ, since this is
what "man 2 times" says you should use.
2. Personally, I'd be just as fine with the original patch that doesn't
have the code complexity of the value caching, but I'm fine with
anything that fixes the bug. Keeping the old behaviour where possible
"for old time's sake" seems a bad idea -- there were at least two broken
platforms (Mac OS and Xeon), and there may be others. Using the
documented behaviour (sysconf) where available is a much better
solution; honestly, sticking to using HZ as a default without support
for that from any documentation has a cargo-cult programming smell to me.

I don't know if there are platforms that have times, but neither sysconf
nor HZ. That sounds very strange to me, but of course I can't rule it
out. But if there are, it is likely that os.times was broken before on
these platforms -- it was broken on two platforms that I wouldn't
consider minor. In that case, it will still be broken, but at least now
we have a unit tests that detects this.

In the first line of my previous message, I mean "#define HZ 60", of course.

Guys, please don't waste too much time on this issue! The tracker still
has more than 1,700 open issues for to debate on. While I enjoy the
fact, that you three are enthusiastic, I strongly feel the urge to
re-route your men power. This bug isn't important enough to waste your
precious time on it.

My opinion as junior core developer is:

sysconf(_SC_CLK_TCK) is the winner and it should be used instead of HZ
when available. A default value should not be used because it will lead
to wrong data. Wrong results are worse than no results.

Since calls to sysconf seem to cost several CPU cycles "clk_tck" should
be cached somehow. I prefer a local static variable in the function but
a static var on file level is fine, too.

The compilation of Python must not fail. When neither HZ nor sysconf is
available but HAVE_TIMES is defined then the function must not be
included. Either you skip the function plus undef HAVE_TIMES or you add
some code to configure.in that does it earlier. The C89 standard doesn't
define #warn so that not an option, too. But configure is allowed fail
for a broken system.

Christian

Christian, I agree on all points. Alexander's patch posixmodule.diff
satisfies those requirements.

I suggest also adding the unit test from os_times4.PATCH (but not the
changes to posixmodule.c in that patch), as this will help identify
misbehaving platforms in the future.

Thanks Cristian for your intervention.  This bug is clearly an 
aberration: how many GvR reported bugs do you know that stayed open for 
3+ years?

I think posixmodule.diff satisfies all your requirements and was not 
opposed by anyone except yours truly.  Do you need anything else to be 
done before you can accept the patches.  It looks like test_posix4.PATCH 
+ posixmodule.diff should be enough.

Great, we're approaching closure! :-)

One final small thing: As said somewhere above, the allowed discrepancy
in test_posix4.PATCH is a bit too low for machines with only 60 ticks
per second. I uploaded a slightly more generous test_posix5.PATCH instead.

So posixmodule.diff + test_posix5.PATCH.

This is the same as what I mentioned above (posixmodule.diff + only the
unit test from os_times4.PATCH).

How abount also check CLK_TCK? I know it's obsolute but maybe
there are platforms without sysconf() and with CLK_TCK.

I think it's better only to only add another fallback if the unit tests
show that such platforms exist. Avoiding cruft is important, too. After
all, sysconf is a standard POSIX API, and from my (admittedly limited)
research was already available in that form back in 1988.

So my suggestion is to apply the current patches.

I agree with Christian's most recent comment.

However without BDFL intervention I think its too late in the 2.6/3.0 
release cycle to rush this fix in.  It can wait for 2.6.1/3.0.1.

I won't have time to look at it for several days myself, but I'm 
assigning the bug to me so that it doesn't sit idle longer than it 
already has; feel free to steal it if someone else intends to fix it 
fast.

Yes, please move this to 3.0.1 / 2.6.1.  The hard part appears to be
making sure that it compiles *and* is correct on all conceivable
platforms...

I don't know what is "HZ", but if it's "hertz" then a division is 
necessary.

         total_clocks
time = -----------------
       clocks_per_second

otherwise there's no hope.

David,

not sure what you are commenting on. Are you commenting on one of the
patches? The patches do contain those divisions, of course; you can also
run the attached unit test to verify that the patches work for you.

Malte proposed this patch for inclusion into Python 2.5. In its current
form, I'm skeptical about doing so, as it has the potential for breakage.

IIUC, the patch requires that HZ is defined if HAVE_TIMES is defined and
HAVE_SYSCONF is not; plus it requires _SC_CLK_TCK to be defined if
HAVE_SYSCONF is defined. For 2.5, such additional constraints are not
acceptable. Instead, the patch should guarantee compilation of
posixmodule if 2.5.2 allowed compilation, no matter how confused the
system is. If some of the prerequisites are not met, it is OK if either
os.times is absent, or produces bogus results.

Martin,

compilation indeed breaks if sysconf is available but _SC_CLK_TCK is
not. My Unix-foo is not sufficient to confidently say that this is
impossible, so my suggestion is to add defined(_SC_CLK_TCK) to the
condition of this #ifdef branch. For what it's worth, this also appears
to be the way Perl does it (perl.c, lines 384-385):

  #if defined(HAS_SYSCONF) && defined(_SC_CLK_TCK) && !defined(__BEOS__)
      PL_clocktick = sysconf(_SC_CLK_TCK);

In the other case you mention, where neither sysconf nor HZ are
available, the old default of 60 could be used instead. A noisy error
appears safer to me to avoid future similar bugs, but I see that this is
a bad idea for Python 2.5.x.

I'll prepare a modified patch.

OK, modified and simplified patch attached (os_times5.PATCH).

The patch and unit test (in test_posix5.PATCH) apply cleanly against the
trunk. "make test" passes on two machines I tried, including a 64-bit
Linux machine where the new unit test fails without the patch. The
caveat is that with the machines I have, I can't exercise all #ifdef
paths. However, the logic looks simple enough now.

> compilation indeed breaks if sysconf is available but _SC_CLK_TCK is
> not. My Unix-foo is not sufficient to confidently say that this is
> impossible

To make such a statement, one would need knowledge of all operating
system releases that have ever been made, including releases that didn't
make it to the public. It might be that POSIX mandates _SC_CLK_TCK,
but that would be irrelevant, as systems might chose not to comply with
POSIX in this aspect.

> In the other case you mention, where neither sysconf nor HZ are
> available, the old default of 60 could be used instead. 

That would probably be safest, although I could also accept that
os.times becomes unavailable on such a system.

Committed os_times5.patch into 2.5 branch as r67739

Applying this to the other branches still needs to be done.

should the unit test in test_posix5.patch have been removed?  (regardless, 
its still on the bug tracker via the history links)

> should the unit test in test_posix5.patch have been removed?  

I see. I wish that people would a) always provide complete patches in
a single file, and b) delete files themselves that have been superceded
by others. In any case, I have re-attached the file; thanks for pointing
this out.

> I see. I wish that people would a) always provide complete patches in
> a single file, and b) delete files themselves that have been
> superceded by others. In any case, I have re-attached the file;
> thanks for pointing this out.

Regarding b), I did that as far as I could (i.e., deleted my own files
that were superseded).

I left the unit test as a separate file because I wasn't too sure if it
would get incorporated -- it looks a bit flaky because it relies on
timing issues, and it's also one of the longer-running tests as it
spends (comparatively) a lot of time in a busy loop. I wouldn't be
surprised if it breaks on some machines sometimes, especially ones with
low clock granularity.

Thanks for the comment though, I'll keep it in mind in the future.

> I wouldn't be
> surprised if it breaks on some machines sometimes, especially ones with
> low clock granularity.

Now that I look at the test, I see what you mean. I think I will reject
it.

Malte, thanks again for the patch; committed into the various branches
as r68018, r68019, r68020, r68021.

As annunciated, I reject the test; I don't think there is a reasonable
way to test for this bug.

Is this fixed in Python 2.6.1?
We have encountered some problems on both OS X and FreeBSD by using 2.6.1:
http://code.google.com/p/psutil/issues/detail?id=40

> Is this fixed in Python 2.6.1?

Please try to answer this question yourself in the future.
Python 2.6.1 was released on Dec 4th
(http://www.python.org/download/releases/2.6.1/), and the
commits were made on Dec 29 (as seen both in the time stamp
on my message, and the *linked* svnview pages).