StratifyLabs / StratifyOS

A Powerful embedded RTOS for ARM Cortex M microcontrollers
https://stratifylabs.co
Other
114 stars 23 forks source link

aio_error() returning +EINPROGRESS interferes with valid async->nbyte of 119 #193

Closed bander9289 closed 5 years ago

bander9289 commented 5 years ago

https://github.com/StratifyLabs/StratifyOS/blob/b19b0aa69f6924e2cd204692b00e26888225d7c7/src/sys/aio/aio.c#L73

The documented way of using aio is to wait for aio_error() to return something other than EINPROGRESS, but if the length of the read/write is 119, this would never happen.

aio_read(&aiocbp);
while( aio_error(&aiocbp) == EINPROGRESS ){
    usleep(1000); //wait for operation to complete
}
tyler-gilbert commented 5 years ago

aio_error() should return zero if the operation is still in progress which is what it does (as far as I know) even though it may not look like that.

int aio_error(const struct aiocb * aiocbp /*! a pointer to the AIO data struture */){
    if ( (volatile void *)aiocbp->async.buf != NULL ){
        return EINPROGRESS;
    } else {
        return aiocbp->async.nbyte; //this is where the error value is stored in case of failure
    }
}

Once the operation completes, the sysfs_aio_data_transfer_callback() function assigns aiocbp->async.nbyte to zero and makes async->buf null.

But it still might be missing something. If the operation starts successfully but has an error during transfer (and transfers zero bytes) then async.nbyte should hold an ERRNO value.

Updating sysfs_aio_data_transfer_callback() with something like the following should work.

    if( aiocbp->async.nbyte < 0 ){
        aiocbp->async.nbyte = SYSFS_GET_RETURN_ERRNO(aiocbp->async.nbyte);
    } else {
        aiocbp->async.nbyte = 0;
    }
bander9289 commented 5 years ago

Thanks for the explanation.

Returning an error was exactly the case I was having trouble with. The proposed change addresses it.

bander9289 commented 5 years ago

Resolved in StratifyLabs/StratifyOS master