|
This is prompted by comments on http://review.whamcloud.com/#/c/10139 from LU-4961. Use of struct obd_ioctl_data tends to lead to some really opaque, unmaintainable, and nonportable code.
- This is a huge kitchen sink structure with 2 obdos, several unions,...
- It has several embdedded pointers, some of which are 'inline' and therefore invalid when the data crosses from user to kernel space or back. Others are out of line but lack a __user attribute.
- The reader must manually remember that something goes in inlbuf1, something else goes in inlbuf2, and so on.
- As Andreas has pointed out, it interleaves fixed width integer types (__u32,...) with API dependent type (char *).
- Most users of obd_ioctl_data could easily be rewritten to use a fixed size struct, or a struct with a single variable length array at the end.
- Several of the ioctls that use it could just be replaced by reading the right proc files.
Libcfs and LNet have some similar dysfunction.
Tangentially, we need to set some standards about adding new ioctls. For example an ioctl that creates a new inode (like LL_IOC_LMV_SETSTRIPE) must accept a mode_t parameter. Ioctls that accept names must handle them properly. And so on.
|