On Sat, 2017-12-30 at 14:00 +0100, Samo PogaÄ?nik wrote:
> Dne 29.12.2017 (pet) ob 15:40 +0200 je Tanu Kaskinen napisal(a):
> > On Sat, 2017-12-16 at 16:57 +0100, Samo PogaÄ?nik wrote:
> > >
> > > + break;
> > > + }
> > > + } else {
> > > + count += l;
> > > + index += l;
> > > + length -= l;
> > > +
> > > + if (length <= 0) {
> > > + break;
> > > + }
> > > + }
> > > + }
> > > +
> > > + pa_memblock_release(pchunk->memblock);
> > > +
> > > + if (u->fifo_error && count >= 0) {
> > > + pa_log("Recovered from FIFO error");
> > > + u->fifo_error = false;
> > > + }
> >
> > This logic is not right. If in the first loop iteration pa_write()
> > does
> > a partial write and then an error occurs during the second iteration,
> > the code incorrectly thinks that we have recovered from the error.
> > The
> > recovery happens when pa_write() returns a positive number for the
> > first time.
> >
>
> This case is correctly covered, since count always get negative value
> in the case of a fifo error, so immediate recovery can not occure via
> this condition.
Ah, indeed. My mistake.
I can still think of an (unlikely) case that doesn't work quite right,
though:
Let's say an error has occurred and fifo_error is true. Then
pipe_sink_write() is called, and the first pa_write() succeeds, but
does only a partial write. The error should be considered recovered at
this point, but your code doesn't set fifo_error = false. When trying
to write the rest of the chunk, an error happens. Now an error message
should be logged, but since fifo_error is still true, that doesn't
happen.
--
Tanu
https://www.patreon.com/tanuk
