sysctl(9): Booleans: Accept integers to ease knob conversion

[olce]

Summary

In sysctl_handle_bool(), if the output buffer (for the old value) has
room for exactly 4 bytes (sizeof(int)), then output the current boolean
value as an integer rather than a 'uint8_t'. Conversely, if 4 bytes
exactly remain in the input buffer (for the new value), treat them as an
integer and derive the new boolean value from it.

Doing so allows to convert existing integer syscstl knobs that are
interpreted as a boolean into true boolean ones while staying
backwards-compatible.

There is no drawback in doing that as no code currently uses
sysctl_handle_bool() as part of a series of calls to sysctl_handle_*()
functions for (de)serialization of some compound structure. If that
case ever materializes, it can be easily solved, e.g., by creating
a sysctl_handle_bool_strict() variant.

MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation

[olce]

Remove useless initialization of temp_int in the SYSCTL_IN() case.

[olce]

Alternative: Leave sysctl_handle_bool() as is, and create another one, as well as accompanying macros SYSCTL_ADD_xxx() and co., to define booleans compatible with ints. In addition, these could unconditionally output/input an integer, easing back and forth internal transition between int and bool as this would preserve the ABI.

[jhb]

sys/kern/kern_sysctl.c

1668

You could perhaps minimize the diff a bit here by doing:

if (req->newlen - req->newidx == sizeof(int)) {
    int temp_int;

    error = SYSCTL_IN(req, &temp_int, sizeof(temp_int));
    temp = temp_int != 0;
} else
    error = SYSCTL_IN(req, &temp, sizeof(temp);
if (error == 0)
   *(bool *)arg1 = temp ? 1 : 0;

This better mirrors what you did above for the SYSCTL_OUT case IMO.

[jhb]

I think changing sysctl_handle_bool directly is fine.

[olce]

Not doing it now, but going further, in order to facilitate changing the internal type of sysctl knobs that are integers, beyond the case evoked here (int -> bool), without breaking the ABI, we should probably tolerate any type of integer in input and output (really, size of I/O buffers) and convert them to the target (internal) type, raising some errors where appropriate (e.g., if the value is out of range for the target type with the exception of booleans).

sys/kern/kern_sysctl.c

1668

There is in fact a small asymmetry. Here, if SYSCTL_IN() fails, temp_int may not have been set at all, in which case we are processing an uninitialized variable to set temp. We could of course initialize temp_int to a dummy value, but computing temp still remains useless in this case and IMHO that can cause slight confusion. Avoiding that computation requires an additional test of error, which is not needed as currently written. Also, in the output case, there is the difference that the value can come from arg2 instead of *arg1, while here we are always setting *arg1, so factoring out felt much less attractive.

[jhb]

sys/kern/kern_sysctl.c

1668

Eh, the temp value is isn't used if SYSCTL_IN fails, so it doesn't matter, though you could always just initialize temp_int to a constant value like 0 which would be simpler than adding an error check.