Skip to content

Trivial fix to time-of-day #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
knc1 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Trivial fix to time-of-day #35

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc7a039
Remove unused tz argument in gettimeofday call.
janetizzy Feb 1, 2018
78a4c67
Fix calculation of fractional seconds.
janetizzy Feb 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions source/util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,9 @@
* \return The real time clock of the operating system in seconds.
*/
double cloog_util_rtclock() {
struct timezone Tzp;
struct timeval Tp;
int stat = gettimeofday(&Tp, &Tzp);
int stat = gettimeofday(&Tp, NULL);
if (stat != 0)
cloog_msg(NULL, CLOOG_WARNING, "Error return from gettimeofday: %d", stat);
return (Tp.tv_sec + Tp.tv_usec*1.0e-6);
return (Tp.tv_sec + Tp.tv_usec/1.0e-6);
Copy link
Collaborator

@ftynse ftynse Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem correct: dividing the number of microseconds by 1E-6 is equivalent to multiplying it by 1E6 (1 million). Given that tc_usec is an integer, and that the function returns the number of seconds as double, I reckon we indeed want to multiply microseconds by 1E-6.

Copy link
Author

@knc1 knc1 Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field is an integer number of microseconds, the intent is to represent that as a fraction of a second (the field is never enough microseconds to represent a full second or more).

Let's try this the easy way, by hand:
Put a 1 dollar bill on table to your left.
Put 99 pennies on table to your right.
How many dollars do you have on the table? (1.99 or 9901.00)
Did you multiple or divide the number of pennies by a 100 to get the fraction of that dollar?

Copy link
Author

@knc1 knc1 Feb 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the change required is to cast the numbers to double before doing the math.
Otherwise the integer operation will truncate the faction of a second.

Copy link
Collaborator

@ftynse ftynse Feb 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we multiply by one millionth here, not by one million. e-6 means negative power, equivalent to 0.000001 in this case. In your words, we multiplied pennies by 0.01...

We could indeed divide by one million, but there is no point in changing the code that is correct.

Copy link
Collaborator

@ftynse ftynse Feb 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for types, multiplication has precedence so Tp.tv_usec*1.0e-6 is performed first. It implicitly converts to double as the type of the RHS is a double literal. Then, before addition, the Tp.tv_sec also implicitly converts to double because its RHS is now double.

}