Hey,
----- Original Message -----
> On 07/08/2014 04:01 PM, Frank Ch. Eigler wrote:
> > Dave Brolley <brolley@xxxxxxxxxx> writes:
> >
> >> [...]
> >>> typedef struct {
> >>> int version;
> >>> int interrupted; /* could become a flags bitfield? */
> >>> const char *globalOptions; /* and this could be more explicit,
> >>> */
> >>> /* instead of every possibly option being
> >>> */
> >>> /* coerced into one comma-separated
> >>> string? */
> >>> } pmDiscoveryOptions;
> >> [...] I ended up going with this approach. [...]
Thanks for considering the options Dave.
> > Oh well. We should leave a note in the docs & code with warning signs
> > about the complete loss of compiler type checking inherent in this
> > approach, and (in case it can help) make a habit of valgrind-running
> > every app that uses this API.
(from the above and later comments, it appears there is no shortage
of misunderstanding here).
Anyway, one-sided discussion of tradeoffs in API design is not needed
in any man page - just document the behaviour and move along, thanks.
Re man pages, you were probably gonna do this anyway Dave but this new
API could be documented on pmdiscoverservices.3 (pmgetcontexthostname.3
does this, for example) to leverage closely-related discussion there.
> > 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.
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.
Its a minor detail though ... using flags is quite a convenient API
extension mechanism though (and the "resolve" field definitely looks
*alot* like a flag).
> > 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).
cheers.
--
Nathan
|