nathans wrote:
> [...]
>> > There is no point using bitfields for the resolve flag, and reserving
>> > space for future bits, as the top version number will have to serve as
>> > sufficient discriminator between all future pmDiscoveryOption struct
>> > variants.
>> True
>
> Hmm, not quite true re flags...
>
>> - I thought that Nathan was perhaps trying to avoid changing the
>> size of the struct when new boolean options are added
>
> That's right. If new flags are added with "default-to-off" semantics
> (i.e. zero meaning off), then new behaviour can be safely introduced
> with no version change.
"no version (code) change" for actual changes is not per se desirable.
Clients are expected to compile against the then-current pmapi.h
header which contains the then-current version code as a macro. The
clients would not hard-code "1" or any other number. Client source
would not need to be modified even if the post-version# fields were
completely scrambled from version to version.
> This extension-by-flag concept is used fairly often for extending some
> kernel APIs without adding new syscalls, for example; flags to open(2)
> and so on, which do come along from time to time - see the man page
> there, it tends to document "since kernel version X.Y" for such flags.
> We can certainly do the same here.
The open(2) syscall is not a good example. It operates under unusual
constraints (passing only a few parameters total), and flags bits are
usually orthogonal. A much closer analogy to what you're building out
here is ioctl(2), and few people would point to that as anything but a
necessary evil.
>> > All places in libpcp that now accept a discoveryOptions struct must at
>> > minimum assert that ->version == 1, lest an app built against a future
>> > incompatible version is run against this one, leading to a crash.
>
> Nonsense. Structure additions must only be appended, and existing
> fields must forever be honoured - so the crash scenario described
> cannot happen (nor do version checks need to be done at this stage,
> the first version is always valid, forever).
If you choose to invent such "forever" constraints (and add a few more
necessary ones, such as having libpcp never return or write-to such
structs, and possibly asserting these padding-reserved bits == 0),
indeed somewhat less protection may be sufficient. On the other hand,
if you wish to tie your own hands less, and protect users more, you
could add the two-instruction version-number check.
.... or we could back away from this variant-struct method of papering
over ABI/API changes, and go for the simple and type-safe:
pmDiscoverServices(...) # the pcp 3.9.0 API/ABI
pmDiscoverServices2(...) # the impending one, with "resolve?"
and interruptability parameters
pmDiscoverServices3(...) # a future one, unconstrained to be a strict superset
(There's plenty of precedent for this, whether in the realm of unix
syscalls, or the recent __pmLogPutResult* one.)
- FChE
|