Skip to content

Fix Alarm app crashing on buttonpress#816

Merged
JF002 merged 1 commit intoInfiniTimeOrg:developfrom
Riksu9000:fix_alarm_crash
Nov 11, 2021
Merged

Fix Alarm app crashing on buttonpress#816
JF002 merged 1 commit intoInfiniTimeOrg:developfrom
Riksu9000:fix_alarm_crash

Conversation

@Riksu9000
Copy link
Contributor

Fixes #770

This should be fixed before releasing 1.7.0

@Riksu9000 Riksu9000 added this to the 1.7.0 milestone Nov 9, 2021
@mruss77
Copy link
Contributor

mruss77 commented Nov 9, 2021

Thanks, I came in here to open this same PR this morning!

I've been testing the same fix for several days and the alarm is no longer crashing.

@Avamander
Copy link
Collaborator

Why not initialize them all to nullptr?

@Riksu9000
Copy link
Contributor Author

Why not initialize them all to nullptr?

Everything else gets initialized in the constructor and we don't initialize them to nullptr in other screens either.

@Avamander
Copy link
Collaborator

Everything else gets initialized in the constructor and we don't initialize them to nullptr in other screens either.

Do you have any ideas how such uninitialized variables could be better avoided? Any warnings?

@Riksu9000
Copy link
Contributor Author

Everything else gets initialized in the constructor and we don't initialize them to nullptr in other screens either.

Do you have any ideas how such uninitialized variables could be better avoided? Any warnings?

There was a warning that the constructor doesn't initialize those variables.

@Avamander
Copy link
Collaborator

There was a warning that the constructor doesn't initialize those variables.

What was the name? Maybe it should be turned into an error with -Werror=<name>.

@Riksu9000
Copy link
Contributor Author

There was a warning that the constructor doesn't initialize those variables.

What was the name? Maybe it should be turned into an error with -Werror=<name>.

Oh not a compilation warning but a clang-tidy one. This one https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.html

@ncartron
Copy link

Hi, I had a pre 1.7 DFU without this PR and the Alarm app was constantly crashing.
I confirm that with a DFU which includes that PR, the Alarm app works as expected, no more crash ! <3

@petterhs
Copy link
Contributor

PR is fixing the issue 👍

@JF002
Copy link
Collaborator

JF002 commented Nov 11, 2021

Thanks! Just in time for 1.7! (Initially, I was going to release it sooner, but I was too busy these last few days...).

@JF002 JF002 merged commit 72900ca into InfiniTimeOrg:develop Nov 11, 2021
@Riksu9000 Riksu9000 deleted the fix_alarm_crash branch November 11, 2021 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash when exiting alarm app with button after setting it

7 participants