Obvious bug in /sys/i386/include/bus.h (was: bus_at386.h)

[Hans Petter Selasky]

On Monday 13 June 2005 14:44, Joerg Sonnenberger wrote:
> On Mon, Jun 13, 2005 at 02:12:38PM +0200, Hans Petter Selasky wrote:
> > This is equivalent to:
> >
> > while(--count)
> > {
> >   /* I/O */
> > }
> >
> > which is obviously wrong, because it doesn't check for count equal to
> > zero.
>
> Why do you think it is a bug? It is part of the interface contract and
> useful to avoid an unnecessary check in 99% of the cases.
>


Where is it documented?


This is a bug, because it will break standard FIFO design. Consider the 
following pseudo code:

static void
filter(struct fifo *f)
{
  do {
     if(!f->len)
     {
         if(f->m) ...;

         f->m = get_mbuf();
         if(!f->m) break;

         f->len = m->m_len;
         f->ptr = m->m_data;
     }

     /* f->Z_chip is the maximum transfer length */

     io_len = min(f->len, f->Z_chip);

     bus_space_write_multi_1(t,h,xxx,f->ptr,io_len);

     f->len -= io_len;
     f->Z_chip -= io_len;
     f->ptr += io_len;

  } while(Z_chip);
}

This code is going to crash if "f->Z_chip" or "f->len" is zero. That happens 
when one is trying to send or receive a zero length frame or mbuf. So then 
one can argue wether one should allow zero length frames or not. I argue that 
zero length frames should be allowed, because it enables easy stream 
synchronization: 

For example to signal end of stream, send a frame less than 16 bytes. Maximum 
frame length is 16 bytes. If a stream is exactly 16 bytes long, then one has 
to send one 16 byte frame and one 0 byte frame.

Also zero length frames can be used as a kind of heart-beat.

Adding that extra check for zero transfer length is not going to affect 
performance at all. If one does that using "C", the compiler can optimize 
away that "if(count) ..." when "count" is a constant. Besides the i386 
machine instructions "ins" and "outs" already exhibit that behaviour.

--HPS