sink: guard against zero frame size in free-frames

[lgirdwood]

sink_get_free_frames() divides the free byte count by the frame size, which
is channels * sample_size and can be zero when the channel count is zero
(host-influenced). Return 0 instead of dividing by zero.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Prevents a divide-by-zero in sink_get_free_frames() when the computed frame size is zero (e.g., host provides channels == 0) by returning 0 frames instead.

Changes:

• Compute frame_bytes once in sink_get_free_frames().
• Add an early return when frame_bytes is zero to avoid division by zero.
• Update comments to document why the guard exists (host-influenced inputs).

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[tmleman]

Shouldn't we reject the case where the number of channels is equal to 0?

[lgirdwood]

Shouldn't we reject the case where the number of channels is equal to 0?

We should indeed, which makes me think we need some agentic rules that help define what valid/invalid configurations would look like for audio at a high level. i.e. to be consumed by the code generating agent and by the code reviewing agent.

[kv2019i]

Not sure about fixing this here

[kv2019i]

This is a hot-path call. Should such checks be made at prepare() stage and not on the hot path?

[lgirdwood]

Agreed in principle. Two notes: the guard reuses the frame_bytes value that's already computed one line above and adds a single (well-predicted) branch, so the hot-path cost is essentially nil — the division it protects is the expensive part. But the root cause is channels_count == 0 from base_cfg reaching the audio_stream, and per my reply to Liam there's no central reject for that today. Options: (a) keep this defensive guard, or (b) reject channels_count == 0 once at module/base_cfg validation (a new central check) and drop this. (b) is the cleaner long-term fix but a broader core change. Happy to do (b) if you'd prefer — which way do you want it?

[lgirdwood]

Lets do option B, we can also have a comment here that channels is validated

[lgirdwood]

Done — went with option B. The host channel count is already rejected when zero at module init (module_adapter_ipc4.c, "bound host channels_count at ipc4 init", now in main), so the divisor cannot be zero from that path. Dropped the hot-path guard and left a comment documenting the invariant instead. The commit is now a no-op functionally; it just records why no runtime check is needed here.

[lgirdwood]

Can you share the link for the comment (module_adapter_ipc4.c, "bound host channels_count at ipc4 init", now in main),

[lgirdwood]

We should reject a frame size of 0 bytes before we get to this call, is there a check higher in the stack that validates this ?

[lgirdwood]

I checked — there isn't a central one. sink_set_channels() stores whatever value it's given (no validation), and the only channel check in the params path (comp_buffer.c) is a mismatch check (stream channels vs params->channels), not a zero check. So a host base_cfg with channels_count == 0 currently propagates down to the audio_stream unguarded, which is what reaches this divide. There is no existing higher-stack reject.