Date: Mon, 26 Jul 2004 14:55:23 -0700 (PDT) From: Don Lewis <truckman@FreeBSD.org> To: conrads@cox.net Cc: freebsd-current@FreeBSD.org Subject: Re: Questionable code in sys/dev/sound/pcm/channel.c Message-ID: <200407262155.i6QLtNuZ058373@gw.catspoiler.org> In-Reply-To: <XFMail.20040726163529.conrads@cox.net>
index | next in thread | previous in thread | raw e-mail
On 26 Jul, Conrad J. Sabatier wrote:
> I'm a little perplexed at the following bit of logic in chn_write()
> (which is where the "interrupt timeout, channel dead" messages are
> being generated).
>
> Within an else branch within the main while loop, we have:
>
> else {
> timeout = (hz * sndbuf_getblksz(bs)) /
> (sndbuf_getspd(bs) * sndbuf_getbps(bs));
> if (timeout < 1)
> timeout = 1;
> timeout = 1;
>
> Why the formulaic calculation of timeout, if it's simply going to be
> unconditionally set to 1 immediately afterwards anyway? What's going on
> here?
Hmn, looks bogus to me. I think the intention is to round timeout up to
1 if the result of the formula is zero. The final assignment statement
looks bogus to me. Maybe a too short timeout is the source of this
problem.
It looks like this assignment appeared in rev 1.65.
> Also, at the end of the function:
>
> if (count <= 0) {
> c->flags |= CHN_F_DEAD;
> printf("%s: play interrupt timeout, channel dead\n", c->name);
> }
>
> return ret;
> }
>
> Could it be that the conditional test is wrong here? Perhaps
> we should be using (count < 0) instead?
>
> I don't know. I'm having no small difficulty understanding this code,
> but these two items caught my attention.
I ran into the same problem when I was looking at the code a few days
ago.
BTW, the trace output that was posted showed write() returning 0
immediately before the failure occurred.
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200407262155.i6QLtNuZ058373>