pcp
[Top] [All Lists]

Re: pmServiceDiscoveryInterrupt() commit a8b87e2 et al.

To: "Frank Ch. Eigler" <fche@xxxxxxxxxx>
Subject: Re: pmServiceDiscoveryInterrupt() commit a8b87e2 et al.
From: Dave Brolley <brolley@xxxxxxxxxx>
Date: Tue, 08 Jul 2014 16:19:20 -0400
Cc: Nathan Scott <nathans@xxxxxxxxxx>, pcp@xxxxxxxxxxx
Delivered-to: pcp@xxxxxxxxxxx
In-reply-to: <y0md2df8vse.fsf@xxxxxxxx>
References: <20140619194444.3B03D58015@xxxxxxxx> <53AB0F27.602@xxxxxxxxxx> <1063089485.33910956.1403758262805.JavaMail.zimbra@xxxxxxxxxx> <53AC35B8.3000802@xxxxxxxxxx> <1193390011.34470957.1403829231937.JavaMail.zimbra@xxxxxxxxxx> <1374649635.181830.1404090114319.JavaMail.zimbra@xxxxxxxxxx> <53BC0D60.4000007@xxxxxxxxxx> <y0md2df8vse.fsf@xxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0

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. [...]
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.

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 - I thought that Nathan was perhaps trying to avoid changing the size of the struct when new boolean options are added, but that probably does not matter in light of the version field.

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.
Yes -- I did this in the main entry point (pmDiscoverServicesWithOptions) where I checked that the app does not expect a newer version of the API.

The man/man3/pmdiscoverservices.3 needs an update & documentation for
this new struct (including initialization requirements);
man3/pmservicediscoveryinterrupt.3 should be removed.
Yep - just waiting for the dust to settle.

- FChE

<Prev in Thread] Current Thread [Next in Thread>