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.

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

Jump to bottom

Crash when calling IRac::sendAc() with SAMSUNG_AC and prev=NULL #1339

Closed
s-hadinger opened this issue Nov 24, 2020 · 5 comments · Fixed by #1341
Closed

Crash when calling IRac::sendAc() with SAMSUNG_AC and prev=NULL #1339

s-hadinger opened this issue Nov 24, 2020 · 5 comments · Fixed by #1341
Assignees
@crankyoldgit
Labels
bug enhancement Pending Confirmation Waiting for confirmation from user

Comments

@s-hadinger
Copy link
Contributor

Version/revision of the library used

2.7.12

Expected behavior

Fix crash sending SAMSUNG_AC

Actual behavior

When calling IRac::sendAc(const stdAc::state_t desired, const stdAc::state_t *prev) with vendor SAMSUNG_AC, there is a crash when prev = nullptr.

The problem happens here: https://github.com/crankyoldgit/IRremoteESP8266/blob/master/src/IRac.cpp#L2535-L2537
calling prev->power without checking first that prev is not null.

One easy fix would be to duplicate the following: https://github.com/crankyoldgit/IRremoteESP8266/blob/master/src/IRac.cpp#L2554-L2555

I let you decide whether you want to add these two lines to SAMSUNG_AC or if you want to add it globally upfront in the code.

Output of raw data from IRrecvDumpV2.ino (if applicable)

NA

What brand/model IR demodulator are you using?

SAMSUNG_AC

Steps to reproduce the behavior

The crash is systematic with Tasmota

Example code used

Circuit diagram and hardware used (if applicable)

I have followed the steps in the [Troubleshooting Guide]

No not needed

Has this library/code previously worked as expected for you?

Yes

Other useful information
@s-hadinger s-hadinger mentioned this issue Nov 24, 2020
Closed
15 tasks
@crankyoldgit crankyoldgit self-assigned this Nov 24, 2020
@crankyoldgit
Copy link
Owner

Acknowledged. I'll add something to make it safe soon.

@s-hadinger s-hadinger mentioned this issue Nov 24, 2020
Merged
6 tasks
crankyoldgit added a commit that referenced this issue Nov 24, 2020
@crankyoldgit
[Bug] Fix crash when IRac::sendAc(state_t, *state_t) called with NULL…
2410efa 
… ptr and SAMSUNG_AC.

* Ensure protocols that use the`prev` pointer's power state are okay when it's NULL.

Fixes #1339
@crankyoldgit crankyoldgit mentioned this issue Nov 24, 2020
Merged
@crankyoldgit crankyoldgit added the Pending Confirmation Waiting for confirmation from user label Nov 24, 2020
@crankyoldgit
Copy link
Owner

crankyoldgit commented Nov 24, 2020 •
edited
Loading

@s-hadinger Should be fixed when PR #1341 is merged.
I'll look at creating a new release when it is.
In the meantime, can you please confirm this fixes your issue?
My tests indicate it does, but extra confirmation from the bug reporter is always best. ;-)

X-ref: arendst/Tasmota#9969

crankyoldgit added a commit that referenced this issue Nov 25, 2020
@crankyoldgit
[Bug] Fix crash when IRac::sendAc(state_t, *state_t) called with SAMS…
71f42c6 
…UNG_AC & NULL (#1341)

* Ensure protocols that use the`prev` pointer's power state are okay when it's NULL.

Fixes #1339
@s-hadinger
Copy link
Contributor Author

Thanks. I did a quick fix in Tasmota and will wait for the next release of your lib. No urgency.

@crankyoldgit
Copy link
Owner

Thanks. I did a quick fix in Tasmota and will wait for the next release of your lib. No urgency.

I saw, but I want to make sure my fix fixes your problem so when it replaces your fix I know everything is still fine. ... before I publish the new release (probably in less than 24h)

crankyoldgit added a commit that referenced this issue Nov 25, 2020
@crankyoldgit
v2.7.13 release
e9b6f32 
_v2.7.13 (20201125)_

**[Bug Fixes]**
- Fix crash when `IRac::sendAc(state_t, *state_t)` called with SAMSUNG_AC & `NULL` (#1341 #1339)
- Mitsubishi112 & 136: `setSwingV()` incorrectly sets mode. (#1337)
- Typo preventing RC6 from compiling when other protocols disabled. (#1332 #1331)

**[Features]**
- Coolix: Improve Sensor(ZoneFollow) and add Vane Step support. (#1340 #1318)

**[Misc]**
- refactor ir_Coolix (#1340)
- refactor ir_Mitsubishi (#1336)
- refactor ir_MitsubishiHeavy (#1333)
@crankyoldgit crankyoldgit mentioned this issue Nov 25, 2020
Merged
crankyoldgit added a commit that referenced this issue Nov 25, 2020
@crankyoldgit
v2.7.13 release (#1343)
d02bc1c 
## _v2.7.13 (20201125)_

**[Bug Fixes]**
- Fix crash when `IRac::sendAc(state_t, *state_t)` called with `SAMSUNG_AC` & `NULL` (#1341 #1339)
- Mitsubishi112 & 136: `setSwingV()` incorrectly sets mode. (#1337)
- Typo preventing RC6 from compiling when other protocols disabled. (#1332 #1331)

**[Features]**
- Coolix: Improve Sensor(ZoneFollow) and add Vane Step support. (#1340 #1318)

**[Misc]**
- refactor ir_Coolix (#1340)
- refactor ir_Mitsubishi (#1336)
- refactor ir_MitsubishiHeavy (#1333)
@crankyoldgit
Copy link
Owner

FYI, the changes mentioned above have now been included in the new v2.7.13 release of the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@crankyoldgit crankyoldgit

Labels
bug enhancement Pending Confirmation Waiting for confirmation from user
Projects
None yet
Milestone
No milestone
2 participants
@crankyoldgit @s-hadinger