Skip to content

Second change synchronization #103

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
sbohmann wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Second change synchronization #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
83 changes: 80 additions & 3 deletions ttyclock.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ update_hour(void)
int ihour;
char tmpstr[128];

ttyclock.lt = time(NULL);
Copy link
Copy Markdown
Collaborator

@anarcat anarcat Jan 13, 2022

Choose a reason for hiding this comment

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

i think there's still a problem here: we remove the lt initialization which means lt will be undefined in the struct, which can lead to undefined behaviour (UB) which we never want. let's set it to something, even if we overwrite it later, it won't hurt, right?

Copy link
Copy Markdown
Author

@sbohmann sbohmann Jan 14, 2022

Choose a reason for hiding this comment

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

Btw. the reason to not have it here is because without some kind of ugly flag, update_hour doesn't know whether this is the first time the timestamp is fetched, in which case rounding it is generally prohibited because the initial display of time must show the non-rounded time at startup.

ttyclock.tm = localtime(&(ttyclock.lt));
if(ttyclock.option.utc) {
ttyclock.tm = gmtime(&(ttyclock.lt));
Expand Down Expand Up @@ -256,7 +255,7 @@ draw_clock(void)
draw_number(ttyclock.date.hour[0], 1, 1);
draw_number(ttyclock.date.hour[1], 1, 8);
chtype dotcolor = COLOR_PAIR(1);
if (ttyclock.option.blink && time(NULL) % 2 == 0)
Copy link
Copy Markdown
Collaborator

@anarcat anarcat Jan 13, 2022

Choose a reason for hiding this comment

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

... specially since we're using it here directly, probably before initialization!

if (ttyclock.option.blink && ttyclock.lt % 2 == 0)
dotcolor = COLOR_PAIR(2);

/* 2 dot for number separation */
Expand Down Expand Up @@ -425,7 +424,7 @@ key_event(void)
{
int i, c;

struct timespec length = { ttyclock.option.delay, ttyclock.option.nsdelay };
struct timespec length = sleep_length();

fd_set rfds;
FD_ZERO(&rfds);
Expand Down Expand Up @@ -679,11 +678,89 @@ main(int argc, char **argv)
update_hour();
draw_clock();
key_event();
ttyclock.lt = timestamp();
Copy link
Copy Markdown
Collaborator

@anarcat anarcat Jan 13, 2022

Choose a reason for hiding this comment

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

maybe initializing it earlier would get rid of this seemingly stray instruction as well?

}

endwin();

return 0;
}

time_t timestamp(void)
{
if (whole_seconds_delay())
{
return rounded_timestamp();
}
else
{
return time(NULL);
}
}

time_t rounded_timestamp(void)
{
static const long HALF_SECOND_IN_NANOSECONDS = 500 * 1000 * 1000;
struct timespec currentTime;

if (clock_gettime(CLOCK_REALTIME, &currentTime) != 0)
{
return time(NULL);
}

if (currentTime.tv_nsec >= HALF_SECOND_IN_NANOSECONDS)
{
return currentTime.tv_sec + 1;
}
else
{
return currentTime.tv_sec;
}
}

struct timespec sleep_length(void)
{
static const long SECOND_IN_NANOSECONDS = 1000 * 1000 * 1000;
static const long NINE_TENTHS = 900 * 1000 * 1000;

if (whole_seconds_delay())
{
struct timespec currentTime;

if (clock_gettime(CLOCK_REALTIME, &currentTime) == 0 &&
currentTime.tv_nsec > 0)
{
struct timespec result;

// avoiding sleeping for < 1 / 10 of a second
Copy link
Copy Markdown
Collaborator

@anarcat anarcat Jan 13, 2022

Choose a reason for hiding this comment

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

nice try, but "why?" why do we avoid sleeping for a tenth? not worth it? how do we handle it in that case?

Copy link
Copy Markdown
Author

@sbohmann sbohmann Jan 14, 2022

Choose a reason for hiding this comment

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

Will amend the comment

if (currentTime.tv_nsec < NINE_TENTHS) {
result.tv_sec = ttyclock.option.delay - 1;
}
else
{
result.tv_sec = ttyclock.option.delay;
}

result.tv_nsec = SECOND_IN_NANOSECONDS - currentTime.tv_nsec;
return result;
}
}

return simple_sleep_length();
}

struct timespec simple_sleep_length(void)
{
struct timespec result;
result.tv_sec = ttyclock.option.delay;
result.tv_nsec = ttyclock.option.nsdelay;
return result;
}

bool whole_seconds_delay(void)
{
return ttyclock.option.delay >= 1 &&
ttyclock.option.nsdelay == 0;
}

// vim: expandtab tabstop=5 softtabstop=5 shiftwidth=5
5 changes: 5 additions & 0 deletions ttyclock.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ void set_second(void);
void set_center(bool b);
void set_box(bool b);
void key_event(void);
time_t timestamp(void);
time_t rounded_timestamp(void);
struct timespec sleep_length(void);
struct timespec simple_sleep_length(void);
bool whole_seconds_delay(void);

/* Global variable */
ttyclock_t ttyclock;
Expand Down