On 06/06/2014 05:12 PM, Josselin Costanzi wrote:
2014-06-06 16:38 GMT+02:00 Lars-Peter Clausen <lars@xxxxxxxxxx>:
On 06/06/2014 04:30 PM, Josselin Costanzi wrote:
Currently the IIO buffer blocking read only wait until at least one
data element is available.
But that is how it is supposed to work. With this patch for example you won't be able to read the last data from the buffer after the buffer has been disabled.
I don't understand the usecase this patch breaks... If the buffer is
disabled then we return what was read alreay.
If there is less data in the buffer than the amount of requested data you'll
keep looping forever.
Or if for example n is not aligned to the sample size you'll also continue to loop forever.
If n isn't aligned to the sample size, wouldn't iio_read_first_n_kfifo
still return data multiple of samples size? In that case we would copy
complete elements until we try to do a short read which would fail at
n < kfifo_esize(&kf->kf) (in iio_read_first_n_kfifo).
The read_first_n() callback will make sure that it only returns full
samples, which means if n is smaller than the sample size it will return 0.
Now in your patch you have
do {
ret = read_first_n(..., n, ...);
n -= ret;
} while (n > 0);
So if n is not a multiple of the sample size you'll end up with n being
smaller than the size of one sample but larger than 0 and at that point
read_first_n() will always return 0 and n will stay at its current value.
This means the exit condition of the while loop will never be met and you
keep on looping forever.
Maybe start with which problem you are trying to address with this patch and
then we can work forward from there to a solution. The current form of the
patch changes the semantics of read() in a way they shouldn't be changed, so
this is not only about the implementation bugs.
- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
