Skip to content

REFAC: remove unnecessary calls to c_str

Victor Poughon requested to merge 513-trivial-cstr into develop

Summary

Issue #513 (closed) points out that calls to c_str are getting more dangerous with C++11 and can causes crashes. We need to identify incorrect uses. But the problem is that there are currently >1500 calls to c_str in OTB, making it difficult to find those potential dangling pointers. However in a lot of cases they are not necessary because there exists a function that takes a std::string directly. We are further helped by C++11 which added some constructors and methods that take strings, where previously only the const char* version existed.

This MR removes 755 calls to c_str which can simply be deleted and need no other refactoring, because a string overload exists. Other instances will require a bit more manual refactoring so they can be done in a future MR. This is only for the very easy ones that can be fixed with sed.

Rationale

Issue #513 (closed).

Implementation Details

Script used:

# Use C++11 sstream constructor from string
# Exclude 'ThirdParty' because it uses ossimFilename type
find Modules/ Examples/ -not -iwholename "*ThirdParty*" -type f -print0 | xargs -0 sed -i -E "s/std::ifstream (.*)\((.*)\.c_str\(\)/std::ifstream \1\(\2/"
find Modules/ Examples/ -not -iwholename "*ThirdParty*" -type f -print0 | xargs -0 sed -i -E "s/std::ofstream (.*)\((.*)\.c_str\(\)/std::ofstream \1\(\2/"

# Use C++11 ::open(const std::string&) of ifstream, ofstream, etc.
# Exclude ThirdParty because of ossimFilename type
find Modules/ Examples/ -not -iwholename "*ThirdParty*" -type f -print0 | xargs -0 sed -i -E "s/\.open\((.*).c_str\(\)/\.open\(\1/"

# No need for c_str when outputing to std::cout and such
find Modules/ Examples/ -type f -print0 | xargs -0 sed -i -E "s/\.c_str\(\) ?<</ <</"

# Use itk::ExceptionObject::SetDescription and SetLocation string versions
find Modules/ Examples/ -type f -print0 | xargs -0 sed -i -E "s/e\.SetDescription\((.*)\.c_str\(\)\)/e\.SetDescription\(\1\)/"
find Modules/ Examples/ -type f -print0 | xargs -0 sed -i -E "s/e\.SetLocation\((.*)\.c_str\(\)\)/e\.SetLocation\(\1\)/"

# Use itksys::SystemTool:: string versions
find Modules/ Examples/ -type f -print0 | xargs -0 sed -i -E "s/itksys::SystemTools::(.*)\((.*)\.c_str\(\)/itksys::SystemTools::\1\(\2/"
# Exclude otbLogger because it uses GetCurrentDateTime which does not have a string version
git checkout --  Modules/Core/Common/src/otbLogger.cxx
# Exclude test/otbOGRLayerStreamStitchingFilter.cxx because there's a double use which is not compatible with the regex
git checkout -- Modules/Segmentation/OGRProcessing/test/otbOGRLayerStreamStitchingFilter.cxx

Copyright

The copyright owner is CNES and has signed the ORFEO ToolBox Contributor License Agreement.

*Check before merging:* - All discussions are resolved - At least 2 👍 votes from core developers, no 👎 vote. - The feature branch is (reasonably) up-to-date with the base branch - Dashboard is green - Copyright owner has signed the ORFEO ToolBox Contributor License Agreement
Edited by Victor Poughon

Merge request reports