Mailing List Archive

[MythTV/mythtv] c370f5: Revert "Potential memory corruption problems in au...
Branch: refs/heads/master
Home: https://github.com/MythTV/mythtv
Commit: c370f513cce8f4bb01d7bada7a10e2dcfeae3e24
https://github.com/MythTV/mythtv/commit/c370f513cce8f4bb01d7bada7a10e2dcfeae3e24
Author: Klaas de Waal <klaas@kldo.nl>
Date: 2022-08-13 (Sat, 13 Aug 2022)

Changed paths:
M mythtv/libs/libmyth/audio/audiooutputbase.cpp

Log Message:
-----------
Revert "Potential memory corruption problems in audiooutputbase"

This reverts commit 81aeb49c456ff4ecab82c2297b22c38bd70d706d.

Rationale:
This commit creates problems with recognition of 5.1 audio devices, see the thread
in mythtv-users "Digital audio Capability grayed out" from James Abernathy.

The problem is that the field "Digital Audio Capabilities" is always disabled
(shown as grey and not accessible) independent of the selected audio device.
Correct behavior is that when a digital output such as HDMI is selected that
it is then possible to select the capabilities of that digital output.
This is not possible when the field is disabled.

I see the following two changes in this commit:
Around line 198 the "bugfix: don't allocate".
The original code creates a copy of the complete struct pointed to by m_outputSettingsDigitalRaw; then
the Users are added and that is then saved in m_outputSettingsDigital.
This means the m_outputSettingsDigitalRaw and m_outputSettingsDigital are always different addresses.

The "bugfix: don't allocate" code changes this by copying the pointer value from m_outputSettingsDigitalRaw, then
the Users are added and then it is saved in m_outputSettingsDigital.

This does change two things:
- The m_outputSettingsDigitalRaw and m_outputSettingsDigital now point to the same struct, so the pointer values are the same.
- The m_outputSettingsDigitalRaw has been modified by adding the users, so it is not the Raw data anymore.
Note that if the idea is that m_outputSettingsDigital and m_outputSettingsDigitalRaw point to the same data then one pointer would be enough.
My conclusion is that this must be the change that causes the device recognition problems.

The "bugfix: don't allocate" code then causes problems in the destructor where now m_outputSettingsDigitalRaw
and m_outputSettingsDigital point to the same object and have the same value.
To protect against twice deleting the same memory the code starting at line 93
"// These all seem to be the same pointer, avoid freeing multiple times"
has been added.

To summarize my analysis:
- The "bugfix: don't allocate" code does create the problem in device recognition by copying a pointer instead of copying a struct.
- The fix in the destructor is only needed as a consequence of this.

Reverting the commit and then checking the behavior of the code with valgrind does not show memory corruption problems.


_______________________________________________
mythtv-commits mailing list
mythtv-commits@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-commits