Why does the original CString get overwritten when passing a copy to the DrawText function with the DT_MODIFYSTRING option?

Question

I've already found a workaround to this problem, but was just wondering if anyone knew what was actually happening to cause the problem I was seeing. My guess is that it has something to do with mutability of strings, but I thought the CString object accounted for that in the copy constructor.

The following code causes mFileName to be overwritten:

class File {
public: 
 ...
 CString GetFilename() {return mFileName;}
private:
 CString mFileName;
};

class FileContainer {
private: File* mFile;
public:
 FileContainer() {
  mFile = new File("C:\temp.txt");
}
 GetFilename(CString& fileName) {
  fileName = mFile->GetFileName();
 }
}

void UpdateText() {
FileContainer fileCnt;
CString filePath(L"");
this->fileCnt.GetFilename(filePath);
...
::DrawText(hDC, filePath, -1, &destRect, DT_PATH_ELLIPSIS | DT_MODIFYSTRING | DT_CALCRECT);
}

What happens is that the first time UpdateText is called, GetFilename returns C:\temp.txt. Assuming that the bounding rect caused the text to be truncated to "...\temp.txt" on the first call, "...\temp.txt" is what is returned from GetFilename on the second call to UpdateText.

Even more perplexing is that this didn't cause mFileName to be changed:

void UpdateText() {
FileContainer fileCnt;
CString filePath(L"");
this->fileCnt->GetFilename(filePath);
filePath = L"TEST";
}

GetFilename always returned C:\temp.txt. So it would seem that the DrawText function is somehow finding the original CString and modifying it. But how?

UPDATE: I figured I'd throw another odd chunk of code that also causes mFileName to be overwritten:

class File {
public: 
 ...
 CString GetFilename() {return CString(mFileName);}
private:
 CString mFileName;
};

That seems like it should create a new object and return that new object. Yet, somehow, DrawText still overwrites mFileName.

If I change the code to the following, I don't have any issues:

class File {
public: 
 ...
 CString GetFilename() {return CString(mFileName.GetBuffer());}
private:
 CString mFileName;
};

The only thing that seems to solve the problem is to construct a new CString the way I showed in the workaround. What is DrawText doing when I pass the DT_MODIFYSTRING option?

Answer 1

Well there are some discrepancies in the code that you posted:

In 'class File':

GetFileName() {return mFileName;}

There is no return type here? Also in the FileContainer class you define the stored 'File' object as a pointer, but in the GetFileName function you access it as if it was not a pointer?

File* mFile;
...
mFile.GetFileName();

As far as why this is happening well right now I can't really tell. Another work around however would be to change the GetFileName function to return a const ref, this should ensure that the returned value can never be changed.

const CString& GetFileName() { return mFileName; }

Answer 2

First, note that CString can be used as a raw string pointer in two ways:

1. operator LPCSTR - gives a pointer which should never be modified.
2. GetBuffer - gives a pointer to memory specifically for the purpose of modifying the string.

Now, DrawText is declared to accept a LPCSTR. So when you pass a CString object directly as in your code, it implicitly uses operator LPCSTR to give the function what it says it wants, a constant string pointer.

However, DT_MODIFYSTRING says that DrawText can modify the string it was given. So internally, DrawText must be throwing away the constness of the pointer and modifying the string anyway.

This combination is a bad thing. But the fault is mainly in the implmentation of DrawText which is violating its own declaration.

As for why this modifies other CString objects: Apparently when a CString object is copied, it delays copying the internal string memory until something tries to modify the string through a CString member function. But until that happens, the operator LPCSTR of each CString object would still point to the same shared internal memory. This is normally fine, as long as any code using it is obeying the rules of const-correctness. However, as we've already seen, DrawText with DT_MODIFYSTRING is not playing by the rules. Thus, it is overwriting memory shared by multiple CString objects.

So to fix this problem, you either need to stop using DT_MODIFYSTRING if you don't actually need the modified text. Or else you need to pass the string to DrawText using filePath.GetBuffer() and then call filePath.ReleaseBuffer() afterwards.