On Wed, 05 Mar 2014, Andreas Gustafsson wrote:
Changing the types of existing sysctl variables breaks both source and binary compatibility and should not be done lightly;
Changing the types without sufficient care can break source and binary compatibility. With sufficient care, compatibility can be maintained, and I thought that dsl had attempted to do that. However, I think that a change like that should have been discussed first.
that's why we have both both hw.physmem and hw.physmem64, for example.
I think that it was a mistake (made several years ago) to introduce hw.physmem64, and that hw.physmem should have been extended to allow larger values, in a backward compatible way, very much like whay dsl has attempted to do now. Some details might be wrong, and it should have been discussed first, but the principle of returning what the caller expects seems reasonable to me.
I believe the harm caused by the incompatible type change far outweighs the cost of a few added lines of code, and would like the original types to be restored.
I agree that an incompatible type change would be harmful. If that problem exists, then it could be fixed either by changing the types back, or by fixing the compatibility. Without knowing the reason for the type change, I don't know which of those options I prefer in the long term. For the short term, read on.
2. I also object to the change of kern_syctl.c 1.247.This change attempts to work around the problems caused by the changes to the variable types by making sysctl() return different types depending on the value of the *oldlenp argument.IMO, this is a bad idea. The *oldlenp argument does *not* specify the size of the data type expected by the caller, but rather the size of a buffer. The sysctl() API allows the caller to pass a buffer larger than the variable being read, and conversely, guarantees that passing a buffer that is too small results in ENOMEM.Both of these aspects of the API are now broken: reading a 4-byte CTLTYPE_INT variable now works for any buffer size >= 4 *except* 8, and attempting to read an 8-byte CTLTYPE_QUAD variable into a buffer of less than 8 bytes is now guaranteed to yield ENOMEM *except* if the buffer size happens to be 4. IMO, this behavior violates both the letter of the sysctl() man page and the principle of least astonishment.Also, the work-around is ineffective in the case of a caller that allocates the buffer dynamically using the size given by an initial sysctl() call with oldp = NULL.
Yes, I think you are right about the details, and we should probably revert the change, at least until a design is discussed that meets all reasonable requirements for compatibility.
--apb (Alan Barrett)