[PATCH] staging:iio: Add wrapper functions around buffer access ops

[Greg KH]

On Wed, Dec 14, 2011 at 06:35:25PM +0100, Lars-Peter Clausen wrote:
> On 12/14/2011 04:49 PM, Greg KH wrote:
> > On Wed, Dec 14, 2011 at 11:15:49AM +0100, Lars-Peter Clausen wrote:
> >> On 12/14/2011 12:59 AM, Greg KH wrote:
> >>>
> >>>>>> +static inline int buffer_get_length(struct iio_buffer *buffer)
> >>>>>> +{
> >>>>>> +	if (buffer->access->get_length)
> >>>>>> +		return buffer->access->get_length(buffer);
> >>>>>> +
> >>>>>> +	return -ENOSYS;
> >>>>>
> >>>>> Here you return an error, but why ENOSYS?
> >>>>>
> >>>>> Consistancy is key, and you don't have it here at all.  Or if you do, I
> >>>>> sure don't understand it...
> >>>>
> >>>> Well, different types of functions require different semantics. While the
> >>>> previous ones did either return 0 in case of success or a error value in case
> >>>> of an error, buffer_get_length returns an integer value where 0 is a valid
> >>>> value. Since we can't make any meaningful assumptions about the buffer size if
> >>>> the callback is not implemented we return an error value. Why ENOSYS? Because
> >>>> it is the code for 'function not implemented' and is used throughout the kernel
> >>>> in similar situations.
> >>>
> >>> Is the caller always supposed to check this?  If so, please mark the
> >>> function as such so the compiler will complain if it isn't.
> >>
> >> Marking the function as __must_check doesn't make much sense here. Since it
> >> will either return an error or the buffer length. So you'll always use the
> >> returned result one way or the other.
> > 
> > That's exactly the point, you must use it, so mark it as such.
> > 
> So by that logic all functions without side effects should be marked as
> __must_check?

"Without side effects"?  What do you mean by this?

Any function, whose return code MUST be checked, should be marked with
__must_check, it's quite simple.

greg k-h