On 2026/03/04 20:17, Christian Schoenebeck wrote:
On Wednesday, 4 March 2026 11:47:01 CET Akihiko Odaki wrote:
On 2026/03/04 17:36, Christian Schoenebeck wrote:
On Wednesday, 4 March 2026 07:16:58 CET Akihiko Odaki wrote:
These functions can be used to re-initialize buffers when hardware
parameters change due to device hotplug, for example.
Signed-off-by: Akihiko Odaki <[email protected]>
Reviewed-by: Phil Dennis-Jordan <[email protected]>
Reviewed-by: Christian Schoenebeck <[email protected]>
---
[...]
+void audio_generic_initialize_buffer_in(HWVoiceIn *hw)
+{
+ g_free(hw->buf_emul);
+ hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+ hw->buf_emul = g_malloc(hw->size_emul);
+ hw->pos_emul = hw->pending_emul = 0;
+}
+
void audio_generic_run_buffer_in(HWVoiceIn *hw)
{
AudioMixengBackendClass *k = AUDIO_MIXENG_BACKEND_GET_CLASS(hw->s);
if (unlikely(!hw->buf_emul)) {
- hw->size_emul = hw->samples * hw->info.bytes_per_frame;
- hw->buf_emul = g_malloc(hw->size_emul);
- hw->pos_emul = hw->pending_emul = 0;
+ audio_generic_initialize_buffer_in(hw);
}
It would be cleaner having split this patch over 2 patches. One for pure
refactoring (no behaviour change), and one that adds g_free(hw->buf_emul);
g_free() was added because the extracted function can be called with
hw->buf_emul set. Extracting this into a function without adding
g_free() makes the function dangerous with non-NULL hw->buf_emul. Adding
g_free() without extracting it into a function doesn't make sense as it
checks !hw->buf_emul immediate above.
You are adding the function. So at this point, nobody could have called the
function by accident except you. And as you would have added g_malloc()
immediately with the next patch, there would be no danger.
In general it does make sense splitting pure refactoring from behaviour
changes. And by adding g_free() you are changing existing behaviour. That's
not bike shedding, it's for the sake of being able to skip all those white
noise changes when investigating if something breaks.
You will also certainly wonder why adding g_free() in that case.
And the potential danger argument contradicts itself, because if the function
was called with an uninitialized structure then g_free() would crash. Without
g_free() it was just a potential memory leak OTOH. Choose your poison.
In the state machine of AudioMixengBackend it is NULL or a valid
pointer: it never has an invalid pointer.
Marc-André Lureau, I'd like to ask you for an opinion on these matters
since you are recently working on this and I think you are eventually
going to make a pull request for this series.
Regards,
Akihiko Odaki
