obsproject
libobs/util: Crash on bmalloc(0)
Description
As outlined in c5965c8605ec8e20ae8a21caf723da94aec62049 (#6721), bmalloc(0) is pretty much always a mistake, possibly hiding other bugs. It's been two years since that commit introduced a warning announcing that this will crash in a future version of OBS, let's make that happen.
cc @notr1ch
Motivation and Context
People are breaking stuff (see other PRs), let's break everything at once. This gives plugin developers who might (unknowingly) do this the opportunity to fix it.
If there are places where we're doing it in OBS itself, this is a great (and certainly loud) way to find out.
How Has This Been Tested?
macOS 15 Launched OBS and played around, did not observe crashes.
Manually added a malloc(0) call, had OBS crash as expected.
Types of changes
- Breaking change (fix or feature that would cause existing functionality to change)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
this is typically reported in pipewire captures, can we fix that first?
To clarify, the "please fix your code! This will crash in future versions of OBS" message currently appears in normal operation using pipewire? That arguably should have been fixed during the last two years, so yes that should be fixed; but in my opinion it shouldn't necessarily hold up this PR - the apparent evidence from the last two years would suggest that it could take ages (seeing that "this will crash in future versions" isn't enough motivation).
https://github.com/obsproject/obs-studio/blob/60a45d3aa3b5900216f0e069e5af901d93013307/plugins/linux-pipewire/pipewire.c#L396-L397 is the only place where it can happen.
https://github.com/obsproject/obs-studio/blob/60a45d3aa3b5900216f0e069e5af901d93013307/plugins/linux-pipewire/pipewire.c#L396-L397 is the only place where it can happen.
I guess the question is whether or not obs_pw_stream->format_info.num is expected to be 0 at times, and if so, should we be calling bzalloc when it is?
https://github.com/obsproject/obs-studio/blob/60a45d3aa3b5900216f0e069e5af901d93013307/plugins/linux-pipewire/pipewire.c#L396-L397
is the only place where it can happen.
I guess the question is whether or not
obs_pw_stream->format_info.numis expected to be0at times, and if so, should we be callingbzallocwhen it is?
Even if it is expected to be zero at times then it needs to be checked and an allocation only take place if is has a >0 value.
As linux-pipewire is an internal module, it needs to be updated in tandem with the change this PR makes, so I'd just add the check and let other "bad" allocations that happen at runtime duly crash the program.
I guess the question is whether or not
obs_pw_stream->format_info.numis expected to be0at times, and if so, should we be callingbzallocwhen it is?Even if it is expected to be zero at times then it needs to be checked and an allocation only take place if is has a
>0value.As
linux-pipewireis an internal module, it needs to be updated in tandem with the change this PR makes, so I'd just add the check and let other "bad" allocations that happen at runtime duly crash the program.
My question was poorly phrased. It should have been "should we actually be calling bzalloc when it is?" or rephrased as a statement such as, "if so, then we shouldn't be calling bzalloc when it is 0."
While I welcome a change to linux-pipewire, I would think that change is out of scope for this PR and would require the expertise of someone who works with the linux-pipewire code, such as @GeorgesStavracas , @kkartaltepe , or @tytan652 .