Skip to content

os_clock() scales a millisecond source with CLOCKS_PER_SEC #11

Description

@mgrossmann

Split out of #8, which fixed the resolution mismatch but deliberately left this in place.

Problem

os_clock() in src/loslib.c scales a millisecond source with a macro that describes
a different clock:

c.u64 = mclock64();                                /* milliseconds */
__64_divmod_u32(&c, CLOCKS_PER_SEC, &sec, &msec);  /* CPU-time macro */
res = ((lua_Number)sec.u32[1]) + (((lua_Number)msec.u32[1])*0.001);

This is correct today only because CLOCKS_PER_SEC happens to be 1000
(libc370/include/time.h:16) — the same number as milliseconds-per-second. The two
values are equal by coincidence, not by definition.

In C, CLOCKS_PER_SEC is the unit of clock(), i.e. CPU time. It says nothing about
mclock64(). If libc370 ever implements clock() for real with a different tick rate,
this divisor silently becomes wrong — the same failure mode as #8, from the other
direction.

Why this is worth doing now

mvslovers/libc370#49 explicitly removed that library's last reader of the macro.
src/time64/tm64time.c now records why:

CLOCKS_PER_SEC describes clock(), which this library does not implement
(src/clib/clock.c returns -1), and is no longer read here.

A grep of libc370/src/ confirms no remaining reader. So lua370's os_clock() is now
effectively the only place in the ecosystem still using CLOCKS_PER_SEC as a
wall-clock scaler — precisely the role libc370 just finished retiring it from.

Fix

Divide by an explicit millisecond constant that means what it says, so the divisor is tied
to mclock64()'s unit rather than to an unrelated macro. Something like:

#define MSEC_PER_SEC 1000
...
__64_divmod_u32(&c, MSEC_PER_SEC, &sec, &msec);

No behavioural change — both are 1000 today. This is about removing a coincidence, not
fixing a current miscalculation.

Note

This becomes moot if #12 is done instead: switching os_clock() to the standard
clock()/CLOCKS_PER_SEC branch would make the macro correct again, because the source
would then genuinely be clock(). Whichever lands first, the other should be closed rather
than both applied.

Ref: #8, mvslovers/libc370#49

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions