search

nimbuscontrols / EIPScanner

Free implementation of EtherNet/IP in C++
https://eipscanner.readthedocs.io/en/latest/
MIT License
238 stars 98 forks source link

Fix msvc compilation #70

Closed JohannesKauffmann closed 2 years ago

JohannesKauffmann commented 2 years ago

While #47 greatly improved the Windows experience, the current master cannot be build with MSVC.

There are three issues here:

  1. VLAs are not supported at all by MSVC (not valid ISO C++)
  2. Designated initializers are C++20 only (GCC currently allows it with compiler extensions)
  3. By default, windows.h #defines min and max, which conflict with std::numeric_limits::min and max.

For 1, I already found it weird that the contents of the input string and vector (CipBaseString constructor and toStdString respectively) are copied into a temporary (VLA) buffer, after which they are copied again, to _data and the returned string. To work around, just use memcpy to copy the contents.

For 2, either the C++ standard needs to be bumped to C++20, or the designated initializers need to be removed (#ifdef-ed out?).

No. 3 is trivial to fix; it requires #define NOMINMAX (but it affects the examples and tests as well).

Let me know what you think.

jadamroth commented 2 years ago

Thank you for the contribution.

I have no problem with this. I'll go ahead and merge and we'll fix with new pull request if it causes any issues

Broekman commented 2 years ago

Hi,

Thanks for the contribution, however this PR breaks the implementation of CipBaseString. E.g. with discovery the product name is now empty.

Vector/string.reserve() only requests a change in capacity, not in size and thus e.g. functions such as .empty() or .size() fail in all where this is used. E.g. vector/string.resize() is required in the constructor and string getter to be able to use size/length functions.

Created a PR #73 - std::copy can be used freely here, both std::memcpy and std::copy implicitly cast char to uint8. Since the vector is always properly resized now, we can safely use begin/end for copy actions (i.e. _length is not used aside from it being required for the ShortString type).