Small fixes#300
Open
izonfreak wants to merge 5 commits into
Open
Conversation
- UTF-8 string literals need to have u8 prefix to them.
- AdsLibTestRef fails to build under linux. The test is only meant to be compile for Windows and FreeBSD systems and should be skiped in all other cases. - Remove warnings and TODOs from FindTcAdsDll.cmake as the case has now been tested and seems to be working. - Fix typos on TwinCAT spelling.
- The variable exception 'e' was unsed in the catch leading to an unsed variable warning. - Instead of only catching std::exception class. Use catch-all (...) handler it catches any exception, regardless of its type. This guarantees that lock is released and events are always notify. The exception is still propagated at the end.
Fix clang-tidy ERR60-CPP rule "Exception objects must be nothrow copy constructable". std::exception is the abstract base class and does not have a constructor that accepts a string. Since you inherit from it the storage of the string message for what() needs to be allocated in the constructor of AdsException in this case using a std::string. Allocating a std::string can theoretically throw (e.g. bad_alloc). If your exception is copy more than once during stack unwinding you will not be able to catch anything. To solve this simply use std::runtime_error which comes with string constructors and noexcept copy constructors. Note here that errorCode should not be a public member of the exception as one also can theoretically, throw the const away and modify the value which is not something you want. However, this is leave "as is" for compatibility reasons.
- Wrong target TcAdsLib was name for target_compile_definitions when building a shared library. It should be AdsLib.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request fixes small issues with cmake and warnings inside the AdsLib. The commits can be squash into a single commit but are currently separated for easier review. See commit messages for a more in-depth explanation of some of the fixes.