search

mdavidsaver / pvxs

PVA protocol client/server library and utilities.
https://mdavidsaver.github.io/pvxs/
Other
19 stars 25 forks source link

Made it build on EPICS 3.14.12.6, all unit tests pass. #3

Closed klemenv closed 4 years ago

mdavidsaver commented 4 years ago

Ha, I didn't know about std::stoul and friends.

Could you also add a travis-ci job for 3.14? This should be as simple as copying the 3.15 job and changing to "BASE=3.14".

https://github.com/mdavidsaver/pvxs/blob/6bb0a364bed580064148031cd76051e74f1b385a/.travis.yml#L71-L78

mdavidsaver commented 4 years ago

I didn't know about std::stoul and friends.

FYI. like strtoul() and friends, the std::sto*() functions ignore extra trailing characters. So eg. given "0 .5", std::stod() happily stops at the space and return zero. The epicsParse*() wrappers have an extra check for this, and return S_stdlib_extraneous. That said, using std::sto*, with appropriate error handling, seems like the way to go.

klemenv commented 4 years ago

I addressed std::sto properly but haven't pushed it yet. I was trying out travis integration and it pointed out a linker problem that I've been fighting since then. Basically tools/info.cpp doesn't link in my epicsParse functions and I can't figure out what's wrong with it. It's both C++ code, but I did try with extern "C" to no avail. I figured it wouldn't hurt to add epicsShareFunc too, although doesn't have any change on Linux/g++. nm finds the functions and obviously they are part of .so since they're used by other functions, so I'm clueless. Any tips?

mdavidsaver commented 4 years ago

I figured it wouldn't hurt to add epicsShareFunc too

You're on the right track. However, try PVXS_API instead of epicsShareFunc. This has the same effect, but doesn't depend on #include ordering.

klemenv commented 4 years ago

With PVXS_API it seems to be just working now. Travis jobs were failing in my local repo too, but succeeded when I restarted them. Not related to this patch.

mdavidsaver commented 4 years ago

Can you have a look at #4? The main difference is to remove use of epicsParse* entirely. Now that I know about them, the std::sto*() family of functions seem like a better fit. Also, as I thought about it. I don't like the idea of libpvxs exporting symbols which normally come from libCom.

mdavidsaver commented 4 years ago

Superseded by #4