Skip to content
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.

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

initialize tm_isdst in System::rtcGetEpoch() #172

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agrif
Copy link

@agrif agrif commented Dec 14, 2022

This member of struct tm was left uninitialized, subject to the whims of whatever garbage was on the stack before it. Normally this isn't a problem, but it can become a problem if you set a timezone with DST. For example:

// in setup
configTzTime("EST5EDT,M3.2.0,M11.1.0", "pool.ntp.org");

// later
display.rtcSetAlarmEpoch(display.rtcGetEpoch() + 60, RTC_ALARM_MATCH_DHHMMSS);

display.rtcGetRtcData();
Serial.printf("now %i %i/%i/%i %i:%i:%i\r\n", display.rtcGetWeekday(), display.rtcGetYear(), display.rtcGetMonth(), display.rtcGetDay(), display.rtcGetHour(), display.rtcGetMinute(), display.rtcGetSecond());

display.rtcReadAlarm();
Serial.printf("alarm %i ..../../%i %i:%i:%i\r\n", display.rtcGetAlarmWeekday(), display.rtcGetAlarmDay(), display.rtcGetAlarmHour(), display.rtcGetAlarmMinute(), display.rtcGetAlarmSecond());

When I run this in my program, with my leftover stack standing in for tm_isdst, I get:

now 3 2022/12/14 5:27:42
alarm 3 ..../../14 4:28:42

and now an alarm I thought I set 60 seconds in the future will never trigger.

This problem disappears when tm_isdst is initialized to -1, which means "infer DST". This will break in the few hours around a DST switchover, but I think this is preferable to breaking arbitrarily based on stack. To avoid the problem completely, don't set a timezone with DST. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant