[linux-dvb] Re: Bug in ring buffer implementation

[Oliver Endriss <o.endriss@gmx.de>]

On Saturday 08 February 2003 04:58, Gregor Lawatscheck wrote:
> At 03:51 08/02/2003, you wrote:
> >Sorry, forget my last message.
> >In this implementation 'free' is not 'ring buffer free count' but
> > 'ring buffer free count'+1.
> >
> >I should not read source code after a long day.
>
> What do you mean? You're certainly onto something here - it certainly
> improves the situation - not to perfect but it does do something that
> appears to be going wrong with the ring buffer.

from dmxdev.c:
        free=buf->pread-buf->pwrite;
        split=0;
        if (free<=0) {
                free+=buf->size;
                split=buf->size-buf->pwrite;
        }
(1)     if (len>=free) {
                dprintk("dmxdev: buffer overflow\n");
                return -1;
        }

Because the check (1) is '>=' and not '>', my first message is bogus...

Well, almost. Similar logic is being used 4 or 5 times in av7110.c.
It is misleading (at least), if ring_buffer_free() does not return 
<number of free> bytes but 1+<number of free bytes>.

As a result ci_ll_reset() does not work correctly:
The check
        if (ring_buffer_free(cibuf)<8)
has to be changed to
        if (ring_buffer_free(cibuf)<=8)
with the current implementation of ring_buffer_free().

There might be more errors due to this. 
I have to check this more carefully.

I think the code should be cleaned up to use a single set of ring buffer 
access routines and not lots of duplicated code which does (almost) the 
same. The file would be easier to understand and much easier to 
maintain. (I volunteer to do this if the driver maintainers are willing 
to accept such changes.)

Oliver


-- 
Info:
To unsubscribe send a mail to listar@linuxtv.org with "unsubscribe linux-dvb" as subject.