Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Details
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. | |