Boost logo

Boost :

Subject: Re: [boost] "peer reviewed" - Rights and responsibilities of maintainers
From: Alexander Grund (alexander.grund_at_[hidden])
Date: 2018-10-18 07:03:22

> const char* impossible = "impossible";
> return impossible !=
> boost::serialization::extended_type_info_typeid<float>::get_const_instance().get_key();
> I memory serves me, get_key() will return the address of string of
> characters inside the the address space of the DLL.  This address is
> compared to the address of "impossible" which is an address inside the
> address space of the calling mainline application.  I could not and
> cannot understand what is being tested here. Honestly, when I see
> this, I felt it unproductive to continue. Given that EVERYONE has
> looked at this and no one has seen any problem with it, I wondering if
> I'm missing something really obvious and I would be grateful if anyone
> could explain this to me.

As Peter noted it is a dummy comparison, its only purpose is to
instantiate 2 different singletons in 2 different shared libraries. #128
does the same but in a more non-obvious way, but in a "valid use" of
Boost.Serialization. #111 is the condensed version of this.

> Hmmm - but the value of the comparison is used in the determination
of the test results.  Very unintuitive.  Why not compare to nullptr (or
0 or whatever) which is what get_key() expected to return since no
entries have been added to the extended type info table.

In response to those concerns raised recently I changed the code and

- use "impossible !=..." to make it clear that the key cannot have this
value and hence the result is always true
 Â  - Thats why I haven't used NULL anymore (I DID use it, but still you
complained, so I made it more explicit that I don't care about WHAT
get_key returns as long as it is called)
 Â  - I have to admit, I wasn't aware what get_key is supposed to return
which is why i return "0" from main if they are NOT "NULL" and the
default "0" if they are. You objected this as it is non-intuitive that
main has an implicit "return 0" and I changed this noticing that get_key
returned "NULL" when I did. To save time trying to find out what
`get_key` is supposed to return I added "impossible"
- add a comment for this: "// Make sure to return true."
- There already was a hint, that different types are required: "// Use a
different(!) type than multi_shared1"
- The purpose is in written in the doc of the cpp file: "library simply
using extended_type_info_typeid"
- All of this is even explained in DETAIL in the main test file
- It is also explained in the PR and the testcase that "This crashes on

Given all this I don't understand what was left so unclear, that you
expected a "useful" test result. The whole purpose of this is to test IF
it runs AT ALL which it currently doesn't. If any description is unclear
and caused the confusion, simply tell me and I can amend it. That would
be constructive and much appreciated.

> There is also an interesting issue of using float and int as type
arguments.  This is never been done before since no user of the
serialization library would do this.  It's an interesting new variable
in the mix and offhand I don't really know if there are any consequences
to this.

Again: They are just used to instantiate the singletons whose
destructions cause the crash. The same can be achieved with custom
classes (see #127) but that makes the code more convoluted than required
for a MWE. If there is an inherent failure in the lib why primitive
types cannot be used, then I didn't found it. I don't see a difference
when passing `int` as opposed to `Foo` from `struct Foo{};` and #127
seems to support this. If this is what bothers you, please comment on
#111 what types I shall pass instead (e.g. Foo and Bar both being simple
structs), but I didn't deem it necessary.

Boost list run by bdawes at, gregod at, cpdaniel at, john at