On Sun, Oct 21, 2012 at 12:53:19PM -0500, Mark Tinguely wrote: > On 10/21/12 09:00, Brian Foster wrote: > >On 10/19/2012 05:02 PM, Mark Tinguely wrote: > >>I am just curious, what is the reason for the padding in the > >>xfs_eofblocks structure? > >> > > > >I added the padding in response to review on an early revision of the set: > > > >http://oss.sgi.com/archives/xfs/2012-09/msg00024.html > > > >The purpose is to allow adding fields to the control structure down the > >road without breaking existing binaries. > > Thank-you for the information. > > I would think that changing the number of arguments would also > involving changing the version number. Yes, it usually does. > The kernel should know > that version 1 copies in 16 bytes, version 2 copies in 16+t bytes, > version n copies in 16+n bytes... Gets messy, pretty quickly. Versioning and padding user facing structures makes life a lot easy when it comes to extending interfaces. 15-20 years of support for an ioctl is a long time, and that's the sort of time frame we need to think about. How to make it easy to maintain and extend over a long period of time. We should always version and pad user facing ioctl structures for this reason. Further, assuming that userspace knows exactly the right size for a given feature is problematic. If there's only one structure type for userspace to use, then it's likely they'll get it right. If there's a different structure for every version of the ioctl, then it's likely they'll get it wrong. And we get validation wrong in the kernel, too. The kernel must validate the size of the structure as being correct given the version number, and that gets hard to validate and easy to get wrong when you have a different structure for every version that exists. See, for example, struct xfs_fsop_geom_t, and XFS_IOC_FSGEOMETRY_V1/ XFS_IOC_FSGEOMETRY. The originaly was an unversioned structure with no padding, and when the V2 log format came along, the structure had to be extended and a new ioctl added to support it. Internally they both used the same struture, but the copy-in/out were different and initialisation was different. The result was leaking unitialised data to userspace because we didn't get it right. (c4d0c3b "xfs: prevent leaking uninitialized stack memory in FSGEOMETRY_V1") At least the new xfs_fsops_geom structure has a version number in it so we don't have to add a new ioctl to extend it further. However, it still doesn't have any padding so any further extensions will have to be very careful to avoid structure size mismatches. This could have been avoided is some padding was added as the time the version number was added.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
