Hi Ken,
----- Original Message -----
> [...]
>
> Done some more digging here.
>
Thanks!
> The change is to e_ext_t defined in libdefs.h, namely int pmda_interface;
> replaced by pmdaInterface *dispatch;. For 32-bit machines, this would not
> change the size of e_ext_t nor the offset of any of the other elements in
> e_ext_t. For other machines, the e_ext_t size _might_ change, but gcc on
> x86_64 the way we use it (default alignment rules in play) will pad the old
> struct so they are exactly the same size and hence the offsets for the other
> elements in e_ext_t are unchanged.
>
> I've checked all this with simple C code.
>
> So we're down to understanding any cases in which an existing binary (not
> part of the PCP package, e.g. an old version of a PMDA that is built from
> source during QA) could believe the interpret the pmdaInterface *dispatch
> value as a int pmda_interface, or vice versa. But libdefs.h is private to
> libpcp_pmda and is not included in any source file other than those in
> libpcp_pmda and libdefs.h is not shipped. And all the instances of e_ext_t
> are calloc'd in open.c within libpcp_pmda.
>
> So I can't see any way that any executable (old or new) can be using e_ext_t
> outside of libpcp_pmda.
*nod* - not intentionally anyway - I couldn't find any accesses outside the
library either, and pmdasimple certainly doesn't access the e_ext_p explicitly
itself.
> Now as to the comment in the header ... when multiple DSO PMDA's are in use
> we need some private-per-PMDA place to hold the skeletal pmResult (biggest
> so far, to avoid malloc/free thrashing) and the hash table of pmids that
> have been requested. So I don't think these have any intersection with the
> changes I made to e_ext_t.
OK.
> And finally on to qa/628 ... this one _rebuilds_ the PMDA from source so
> seems to be immune from any e_ext_t changes.
*nod* - I had thought it might be only one of the two PMDAs involved in the
test was being rebuilt, but yep - both are rebuilt. Its just bizarre.
> But I've just seen a failure!!
Yippee!!!
> Nathan does this match the failure signature you saw?
>
> --- 628.out 2013-11-04 12:16:16.077770453 +1100
> +++ 628.out.bad 2014-12-12 07:30:21.297771237 +1100
> @@ -1,7 +1,7 @@
> QA output created by 628
>
> simple.numfetch PMID: 253.0.0
> - value 3
> +No value(s) available!
>
> idiot.numfetch PMID: 177.0.0
> value 3
> @@ -13,7 +13,7 @@
> value 5
>
> simple.numfetch PMID: 253.0.0
> - value 4
> +No value(s) available!
>
> idiot.numfetch PMID: 177.0.0
> value 6
It certainly does, yes...
commit 6b26d4315270aaa7e572927af23fc2e83e0495b9
Author: Nathan Scott <nathans@xxxxxxxxxx>
Date: Fri Nov 28 16:13:23 2014 +1100
Revert "libpcp_pmda: change e_ext_t to expose pmdaInterface"
This reverts commit f856e2c17103540b87952e396f3a7de9cec9a66a.
Test failure is being observed in qa/628, this commit is the
current prime suspect (not sure why though, as yet):
$ ./check -q -l 628
628 27s ... - output mismatch (see 628.out.bad)
4c4
< value 3
---
> No value(s) available!
16c16
< value 4
---
> No value(s) available!
So, reverting temporarily and will reinstate after pcp-3.10.2
which is pending.
--
Nathan
|