I am trying to modify a program so that when arguments are passed into argv they can be modified and then a new array created and used for execution. At present I am creating a vector of const char* in order to hold the values before building an array.
When I am building the vector any value that I alter is coming out as random data and not the string I am building (I built a string then passed it to the vector with c_str()). Below is the code:
int main(int argc, char const *argv[])
{
std::vector<const char*> args;
for(uint i = 0; i < (uint)argc; i )
{
if(((string)argv[i] == "-t" || (string)argv[i] == "--test") && (uint) argc - 1 > i)
{
string testPath = test::testPath().lexically_normal().string();
if (boost::filesystem::exists(argv[i 1]) && !strstr(argv[i 1], testPath.c_str()))
{
if (boost::filesystem::is_regular_file(argv[i 1]))
{
boost::filesystem::path p(argv[i 1]);
boost::filesystem::path newPath = test::testPath().lexically_normal();
newPath /= "/ImportedTests";
if (!boost::filesystem::exists(newPath.lexically_normal()))
{
if(boost::filesystem::create_directory(newPath.lexically_normal()))
cout << "An error occurred creating the import directory for specified tests at " << newPath << endl;
}
newPath /= p.filename();
cout << "newPath = " << newPath << endl;
if(boost::filesystem::extension(newPath) == ".nuk" || boost::filesystem::extension(newPath) == ".div")
{
if (rename(argv[i 1], newPath.c_str()) != 0)
cout << "Error importing tests." << endl;
}
args.push_back(argv[i]);
args.push_back(newPath.c_str());
cout << "newPath.c_str() = " << newPath.c_str() << endl; // Prints readable output.
i ;
}
else
{
boost::filesystem::path p(argv[i 1]);
boost::filesystem::path newPath = test::testPath().lexically_normal();
newPath /= "/ImportedTests";
if (!boost::filesystem::exists(newPath.lexically_normal()))
{
if(boost::filesystem::create_directory(newPath.lexically_normal()))
cout << "An error occurred creating the import directory for specified tests at " << newPath << endl;
}
for (const auto & entry : fs::directory_iterator(argv[i 1]))
{
if (boost::filesystem::extension(entry.path()) == ".sol" || boost::filesystem::extension(entry.path()) == ".yul" )
{
if (rename(entry.path().c_str(), newPath.c_str()) != 0)
cout << "Error importing tests." << endl;
}
}
args.push_back(argv[i]);
args.push_back(newPath.string().c_str());
cout << "newPath.c_str() = " << newPath.c_str() << endl;
i ;
}
}
}
else
{
args.push_back(argv[i]);
}
}
for(uint i = 0; i < args.size(); i ){
cout << "element " << i << " = " << args[i] << endl; // when new path is printed here it is not readable. It's just random characters.
}
}
I did not write this application so I can't change how argv is passed to the application without causing issues. I just want to build the same type of array and modify values if need be.
The output comes out as:
newPath.c_str() = /home/dev/Development/test/ImportedTests
element 0 = test/tools/isoltest
element 1 = --vm
element 2 = ../../evmone-0.8.0-linux-x86_64/lib/libevmone.so
element 3 = -t
element 4 = Dy8^
If I can get this string properly cast as a const char* I think I can get an array from the args vector with args.data() and then pass that to the rest of the application.
If any of this is just completely stupid I apologize in advance.
CodePudding user response:
args.push_back(newPath.c_str());
The pointer that's returned by c_str()
is owned by the object that it's returned from. Here, this object is newPath
. When this object gets destroyed (or if this object is modified), the returned const char *
is no longer valid.
This newPath
object gets destroyed at the end of its if()
statement, when its scope end. newPath
is declared inside the if()
statement, so it gets destroyed at the end of it. This is how all objects work in C .
The shown code then attempts to fetch out the const char *
out of args
much later, when its owning newPath
is just a distant memory.
This is the reason for the undefined behavior, and the observed garbage.
You must completely re-engineer and fundamentally restructure the shown code's logic so that the objects that own all the const char *
do not get destroyed, as long as this character pointer exists.
A much simpler solution is to get rid of the character pointers completely. Make args
a std::vector<std::string>
, and args.push_back(newPath)
into it. Whenever there's a need to work with the underlying const char *
s, obtain them and use them for no more than they are needed, and without any changes to the underlying vector.
CodePudding user response:
This is undefined behavior. You're storing pointers to string data that belongs to local objects which have been destroyed. If you want to collect those strings, you must copy them.
It's unclear why you don't just store a vector<string>
in the first place. If you absolutely need a vector of pointers, then store the args as std::string
, and then create a separate vector with raw pointers:
// Populate args
std::vector<std::string> args;
for(uint i = 0; i < (uint)argc; i )
{
// ...
}
// Create vector of pointers to args
std::vector<const char*> argptr;
argptr.reserve(args.size());
for (const auto& str : args) argptr.push_back(str.c_str());