Automated file rename - const variable is changing its value?

Question

I'm kind of a noob with the std namespace, and I'm writing code that loops through all the jpeg files in a directory and removes any exclamation points. I'm trying to use std::string and std::vector. My problem is that my variable tempname: const char tempname = (char) *filelist[j].c_str(); changes as the strings in the vector filelist changes (which it shouldn't - it is a constant variable. Here is the meat of my WinMain function:

std::vector<std::string> filelist;
if (!dirExists(directory)) //checks if a directory exists
{
    CreateDirectory("resized", NULL);
}
std::vector<std::string> filelist = findFiles("*.jpg"); //finds files in its directory with a given extension
int result; //for rename function
for (unsigned int j=0; j< filelist.size(); j++)
{
    std::string::size_type pos = filelist[j].find("!"); //check for exclamation points
    if (std::string::npos != pos) //found one at index "pos" in the string
    {
        switch (MessageBox(window, (LPCSTR)filelist[j].c_str(), "Illegal filename - Rename?", MB_YESNO)) //user input
        {
            case IDYES:
            {
                const char tempname = (char) *filelist[j].c_str(); //the problem
                //attempt to remove the exclamation point
                result = rename(&tempname, filelist[j].erase(pos, 1).c_str());
                if (result == 0)
                    MessageBox(window, "Renamed File", "Information", MB_OK);
                else
                    MessageBox(window, "Error renaming file", "Error", MB_OK);
                break;
            }
            case IDNO:
            {
                break;
            }
        }
    }
}

Assuming the filename contains no more than one exclamation point. If I defined tempname as a const char* this would make sense, because it would be a pointer - tempname's value could change without violating the const declaration if the data it pointed to changed. But taking away the pointer, I'm baffled.

"const char tempname" is one char

doctorlove
– doctorlove

2013-07-05 15:33:14 +00:00
Commented Jul 5, 2013 at 15:33 — doctorlove
– doctorlove, Commented Jul 5, 2013 at 15:33

Timo Geusch · Accepted Answer · 2013-07-05 15:32:04Z

You realize that your declaration of tempname means that you'll be copying in exactly one character? I'm pretty sure that's not what you wanted.

You probably want to make a copy of the string itself and change the code as follows:

            std::string const tempname = filelist[j];
            //attempt to remove the exclamation point
            result = rename(tempname.c_str(), filelist[j].erase(pos, 1).c_str());

As to why your supposedly const variable change its value if you manipulate the underlying string, keep in mind that in your original declaration, tempname is a pointer to a value and all you said was that the pointer doesn't change its value. Which it didn't, but the pointee did.

Also, when combining c_str with string manipulation you're pretty much entering dangerous territory - have a look at the documentation for c_str here, it clearly states that calling mutating member functions on the string object may invalidate the result of the c_str() call. You have to be careful when mixing std::string with C string idioms.

James Kanze · Accepted Answer · 2013-07-05 16:52:44Z

The code you post has undefined behavior, which means that anything can happen. You define a single char, tempname, and then pass its address to rename. rename requires a pointer to a '\0' terminated string; the only legal one character string would be an empty string.

If you replace the definition of tempname with:

char const* tempname = filelist[j].c_str();

then you have undefined behavior later; the call to filelist[j].erase invalidates this pointer. In practice, it will in fact continue to point to the data in filelist[j] (since in fact, the erase will not reallocate, and so tempname will continue to point to the first character in filelist[j]).

What you probably want is something like:

std::string newName( filelist[j] );
newName.erase( std::remove( newName.begin(), newName.end(), '!' ),
               newName.end() );
result = rename( filelist[j].c_str(), newName.c_str() );
filelist[j] = newName;

Collectives™ on Stack Overflow

Automated file rename - const variable is changing its value?

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related