Re: [PATCH 04/27] iio: Add iio_push_buffers_with_timestamp() helper

[Lars-Peter Clausen <lars@metafoo.de>]

On 09/16/2013 08:37 PM, Jonathan Cameron wrote:
> On 09/16/13 09:19, Lars-Peter Clausen wrote:
>> On 09/16/2013 09:52 AM, Jonathan Cameron wrote:
>> [...]
>>> Interesting.  Whilst this obviously results in the removal of a lot of
>>> repeated code, I am nervous about introducing the 'hidden' requirement
>>> that the data buffer passed in must be bigger than is 'apparently' used
>>> in the code calling this. I'm not sure what the right answer is though.
>>
>> Well it's not that hidden, it is clearly documented that the function is
>> going to store the timestamp in the buffer.
> But who reads the docs? :)

Hopefully everybody ;)

>> My first idea was to make
>> storing timestamp a separate function. E.g. like
>>
>> iio_store_timestamp(indio_dev, buf, ts);
>> iio_push_to_buffers(indio_dev, buf);
> That was my first thought as well.
>>
>> This makes it a bit more explicit that the buffer needs to be large enough
>> to hold the timestamp. But since that function would always be followed by
>> iio_push_to_buffers() I choose to add a function that does both, store the
>> timestamp and push the buffer out.
> Hmm. I'm more or less convinced though I think moving the buffer allocation
> into the core (or nearly the core) would be a good way of avoiding any
> confusion in the long run.

Things will go bad anyway if your buffer is smaller than scan_bytes as 
iio_push_to_buffers expects this.

We could do something like this, but it is rather ugly:

#define iio_push_to_buffers(indio_dev, buffer) \
({ \
	BUG_ON(sizeof(buffer) > sizeof(void*) && \
		sizeof(buffer) < indio_dev->scan_bytes); \
	iio_push_to_buffers(indio_dev, buffers); \
})

- Lars