On 3/24/21 10:10 AM, Alexandru Ardelean wrote:
On Tue, Mar 23, 2021 at 1:35 PM Jonathan Cameron
<Jonathan.Cameron@xxxxxxxxxx> wrote:
[..]
Continuing a bit with the original IIO buffer ioctl(), I talked to
Lars a bit over IRC.
And there was an idea/suggestion to maybe use a struct to pass more
information to the buffer FD.
So, right now the ioctl() just returns an FD.
Would it be worth to extend this to a struct?
What I'm worried about is that it opens the discussion to add more
stuff to that struct.
so now, it would be:
struct iio_buffer_ioctl_data {
__u32 fd;
__u32 flags; // flags for the new FD, which maybe we
could also pass via fcntl()
}
anything else that we would need?
I have a vague recollection that is is almost always worth adding
some padding to such ioctl data coming out of the kernel. Gives
flexibility to safely add more stuff later without userspace
failing to allocate enough space etc.
I'm curious though, because this feels backwards. I'd expect the
flags to be more useful passed into the ioctl? i.e. request
a non blocking FD? Might want to mirror that back again of course.
The struct can be used for both. Passing flags and buffer number in and fd out.
Personally, I don't know.
I don't have any experiences on this.
So, then I'll do a change to this ioctl() to use a struct.
We can probably add some reserved space?
struct iio_buffer_ioctl_data {
__u32 fd;
__u32 flags;
__u32 reserved1;
__u32 reserved2;
}
What to make sure of when using reserved fields is to check that they are 0.
And reject the ioctl if they are not. This is the only way to ensure that old
applications will continue to work if the struct is updated.
Lars was giving me some articles about ioctls.
One idea was to maybe consider making them multiples of 64 bits.
But reading through one of the docs here:
https://www.kernel.org/doc/html/latest/driver-api/ioctl.html#interface-versions
it discourages to do interface versions.
But I guess if we plan ahead with some reserved space, it might be
somewhat fine.
I'm still a little green on this stuff.
