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)
