[ECOS] probable bug in io/serial/common/serial.c!

[Mohammed Illyas Mansoor <mansoor at cdotb dot ernet dot in>]

Hi,
    I was going through the common serial io code it seems to me
that there probably is a bug in it, below is a snap shot of the code
in question.

<snap>
static Cyg_ErrNo
serial_write(cyg_io_handle_t handle, const void *_buf, cyg_uint32 *len)
{
    cyg_devtab_entry_t *t = (cyg_devtab_entry_t *)handle;
    serial_channel *chan = (serial_channel *)t->priv;
    serial_funs *funs = chan->funs;
    cyg_int32 size = *len;
    cyg_uint8 *buf = (cyg_uint8 *)_buf;
    int next;
    cbuf_t *cbuf = &chan->out_cbuf;
    Cyg_ErrNo res = ENOERR;

    cbuf->abort = false;
    cyg_drv_mutex_lock(&cbuf->lock);
    if (cbuf->len == 0) {
        // Non interrupt driven (i.e. polled) operation
        while (size-- > 0) {
            while ((funs->putc)(chan, *buf) == false) ;  // Ignore full,
keep trying
            buf++;
        }
    } else {
        cyg_drv_dsr_lock();  // Avoid race condition testing pointers
        while (size > 0) {
            next = cbuf->put + 1;
            if (next == cbuf->len) next = 0;
            if (next == cbuf->get) {
                cbuf->waiting = true;
                // Buffer full - wait for space
                (funs->start_xmit)(chan);  // Make sure xmit is running
                cbuf->pending += size;  // Have this much more to send
eventually]
***---->cyg_drv_cond_wait(&cbuf->wait);<-----***
                cbuf->pending -= size;
                if (cbuf->abort) {
                    // Give up!
                    cbuf->abort = false;
                    cbuf->waiting = false;
                    res = -EINTR;
                    break;
                }
            } else {
                cbuf->data[cbuf->put++] = *buf++;
                cbuf->put = next;
                size--;  // Only count if actually sent!
            }
        }
        cyg_drv_dsr_unlock();
        (funs->start_xmit)(chan);  // Start output as necessary
    }
    cyg_drv_mutex_unlock(&cbuf->lock);
    return res;
}
</snap>

    In the line with ***--->cyg_drv_cond_wait(&cond_wait);<----***,
will cause a deadlock, because before going for a wait on the condition
variable
start_xmt function is called which will trigger the transmitter to send
the char's, if the call to xmt_char finds out that chan->waiting is true

and wakes up the writer thread which was supposed to be waiting by now,
he would also change the chan->waiting to false. Later, when the wait by

the writer is called there will be no waking up of the writer by the
driver
(as it has been already done by the xmt_char() ), causing it to
deadlock.

The fix might be as below,

            if (next == cbuf->get) {
                cbuf->waiting = true;
                // Buffer full - wait for space
                (funs->start_xmit)(chan);  // Make sure xmit is running
                cbuf->pending += size;  // Have this much more to send
[eventually]
            ++if ( cbuf->waiting == true)
                cyg_drv_cond_wait(&cbuf->wait);


--With Regards,

M. I. Mansoor.