Page MenuHomeFreeBSD

sound: Call vchan_destroy() on vchan_create() failure
ClosedPublic

Authored by christos on Dec 23 2024, 9:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 8, 10:46 AM
Unknown Object (File)
Sat, May 3, 6:41 PM
Unknown Object (File)
Apr 20 2025, 9:54 AM
Unknown Object (File)
Apr 19 2025, 7:13 AM
Unknown Object (File)
Apr 17 2025, 11:32 PM
Unknown Object (File)
Apr 17 2025, 10:36 AM
Unknown Object (File)
Apr 14 2025, 8:30 PM
Unknown Object (File)
Apr 10 2025, 10:58 PM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/dev/sound/pcm/vchan.c
815

It looks like vchan_destroy() can fail. Should we assert here that it succeeds?

sys/dev/sound/pcm/vchan.c
815

We can, but I don't think it's important. vchan_destroy() is called once the vchan has been added to the children list, and the parent has CHN_F_BUSY set, so vchan_destroy() cannot fail in this case. I am aware this relies on the assumption that goto fail will always be used after those 2 conditions are met, but the alternative is to essentially duplicate vchan_destroy()'s code.

sys/dev/sound/pcm/vchan.c
815

I think that assumption is subtle enough to warrant an assertion, especially to help catch future mistakes. Assertions cost basically nothing anyway.

sys/dev/sound/pcm/vchan.c
815

Thinking a bit more about it, it might be better to modify vchan_destroy() instead so that it doesn't fail. That is, remove the CHN_F_BUSY check (not sure why we want it in the first place), and run the following block only if the list is not empty:

	if (CHN_EMPTY(parent, children))
		return (EINVAL);

	/* remove us from our parent's children list */
	CHN_REMOVE(parent, c, children);

	if (CHN_EMPTY(parent, children)) {
		parent->flags &= ~(CHN_F_BUSY | CHN_F_HAS_VCHAN);
		chn_reset(parent, parent->format, parent->speed);
	}

IMHO vchan_destroy() should be able to call chn_kill() regardless of whether we've added the vchan to the children list or not.

sys/dev/sound/pcm/vchan.c
815
This revision is now accepted and ready to land.Dec 28 2024, 4:15 PM