On 14/06/21 22:01, Martin Nyhus wrote:
On 05/06/2021 02:33, João Valverde wrote:
But regarding your PoC having to give extern linkage to the internal
dissector code is a big drawback IMO, even if it isn't visible in a DLL
(because we use default hidden visibility when the compiler supports it).
Maybe that could be solved by including the dissector source file in the
test source file?
After a few different attempts I've updated the merge request with your
idea, and none of the changes to the dissector code are needed anymore.
The new problem that shows up is that we now build the dissector twice,
once for the normal build and once when included in the test file, and
linking those fails because of the duplication. Unfortunately we can't
just exclude all the dissectors from the test linking as epan depends on
code in quite a few dissectors (enough that I gave up trying to make a
list of them).
Having epan depend on dissectors shouldn't happen, although in practice
I think it has often been accepted as a mostly harmless expedient.
It should be fixable. I'll take a look at that.
The solution I ended up with is to split out the dissectors that are
being tested, and making a new object that doesn't include those. In the
future if someone wants to add tests for a dissector that epan depends
on this won't work, but it should be compatible with the previous PoC of
header + non-static functions, so there are options available.
If someone knows of a way to have Cmake include one object library in
another I'd like to get rid of DISSECTOR_UNTESTED, but for now I haven't
found a way to do it.
Appreciate the feedback,
Martin