radarsat1 / liblo

liblo is an implementation of the Open Sound Control protocol for POSIX systems
GNU Lesser General Public License v2.1
192 stars 60 forks source link

Likely bug in liblo cpp wrapper #152

Closed jcelerier closed 10 months ago

jcelerier commented 11 months ago

Here: https://github.com/radarsat1/liblo/blob/ed21c61f433a663533ce8e87d13c067b9248cf46/lo/lo_cpp.h#L637

std::remove_if only moves the things at the end of the container, but does not erase them (I noticed this thanks to a buildsystem warning in a project in a recent toolchain, [[nodiscard]] ftw :)).

To be correct this code needs either the remove-erase idiom in pre-c++20, or if you can use C++20 the much simpler and clearer way:

for (auto &i : _handlers) {
  std::erase_if(i.second, [&](std::unique_ptr<handler>& h){return h->method == m;});
}

I can provide a patch for whichever method is preferred

JohannesLorenz commented 11 months ago

Thanks for the suggestion to deliver a patch.

The decision whether we can use C++20 will depend on the CI. I am currently busy trying to get it work. Once that is done I would come back to you with that info.

JohannesLorenz commented 10 months ago

Hi again. After some internal discussions with Stephen, we would like to ask you to submit the pre-c++20 approach. Thanks on advance.