We value your feedback.
Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!
Course Of The Month
8 days, 13 hours left to enrollBecome a Premium Member and unlock a new, free course in leading technologies each month.
Solved
MFC CString concatenation failure
Posted on 2008-10-14
If I have a local variable of type CString and then assign to it using the concatenation operator, things work fine:
CString StringFunction1()
{
CString one;
one = "ONE";
return one;
}
CString StringFunction2()
{
CString two;
two = "TWO";
return two;
}
{
CString x;
x = StringFunction1() + StringFunction2();
MessageBox(x);
}
However, if the second function is replaced by a member function of one of my classes,
{
MyClass myobject;
CString x;
x = StringFunction1() + myobject.StringFunction();
MessageBox(x);
}
then no concatenation happens; the messagebox just shows "ONE".
class MyClass
{
public:
MyClass();
CString StringFunction();
};
CString MyClass::StringFunction()
{
return CString("CSF");
}
I've looked on various google pages, and people keep saying you can't do this because you're returning a pointer to a local variable that no longer exists. Why ? Surely "return" has something like operator= semantics, in that it constructs a copy of whatever it is, in the callER's stack space ?
CString StringFunction1()
{
CString one;
one = "ONE";
return one;
}
CString StringFunction2()
{
CString two;
two = "TWO";
return two;
}
{
CString x;
x = StringFunction1() + StringFunction2();
MessageBox(x);
}
However, if the second function is replaced by a member function of one of my classes,
{
MyClass myobject;
CString x;
x = StringFunction1() + myobject.StringFunction();
MessageBox(x);
}
then no concatenation happens; the messagebox just shows "ONE".
class MyClass
{
public:
MyClass();
CString StringFunction();
};
CString MyClass::StringFunction()
{
return CString("CSF");
}
I've looked on various google pages, and people keep saying you can't do this because you're returning a pointer to a local variable that no longer exists. Why ? Surely "return" has something like operator= semantics, in that it constructs a copy of whatever it is, in the callER's stack space ?
[X]
Welcome to Experts Exchange
Add your voice to the tech community where 5M+ people just like you are talking about what matters.
- Help others & share knowledge
- Earn cash & points
- Learn & ask questions
6
5-
2
13 Comments
Worked for me. I got a message box that said "ONCSF". I'm using VS2005.
#include <afxwin.h>
CString StringFunction1()
{
CString one;
one = "ONE";
return one;
}
CString StringFunction2()
{
CString two;
two = "TWO";
return two;
}
class MyClass
{
public:
MyClass() {}
CString StringFunction();
};
CString MyClass::StringFunction()
{
return CString("CSF");
}
int main()
{
MyClass myobject;
CString x;
x = StringFunction1() + myobject.StringFunction();
MessageBox(NULL, x, x, MB_OK);
}
It must work for VC6 as well. Please post the exact code where you got the issue. The above seems to be edited here in EE as it wouldn't compile. I assume your original code has some crudities by using same names for member variables, arguments or global variables or same names for member functions and global functions.
Note, I don't see a value in naming global functions similar or equal to member functions. But if you do so, it is a good chance to shoot yourself into the foot.
Non-static member functions should somehow refer to (non-static) member data. If not make them static functions so that they can be used without an object.
Note, I don't see a value in naming global functions similar or equal to member functions. But if you do so, it is a good chance to shoot yourself into the foot.
Non-static member functions should somehow refer to (non-static) member data. If not make them static functions so that they can be used without an object.
OK. When I create a minimal program such as I posted originally, the concatenation does indeed work correctly. However, my main program still does not. It's impractical to split down the big program into a form suitable for posting here, but I'll do so if I can get it to a form which demonstrates the problem.
In my main program, I have a member function like this:
void CMC2Dlg::update_title()
{
CString t1 = vi.GetProductName() + " - " + GetDocumentTitleBaseName()
#ifdef _DEBUG
t1 = "(DEBUG) "+t1;
#endif
t1 += ( (m_dstate==dirty) ? "*" : "" );
this->SetWindowText(t1);
}
What happens is that the window text gets set to the contents of vi.GetProductName(); everything after that is lost. However, when I change the vi.GetProductName() definition to return a dummy value, it all works correctly.
So, it appears that my member function vi.GetProductName() is doing something weird with the return value.
vi is an instance of the class VersionInformation, which I created. It looks like this:
I seem to remember having a problem with const-ness in this function; I had to make something non-const even though it was const.
In my main program, I have a member function like this:
void CMC2Dlg::update_title()
{
CString t1 = vi.GetProductName() + " - " + GetDocumentTitleBaseName()
#ifdef _DEBUG
t1 = "(DEBUG) "+t1;
#endif
t1 += ( (m_dstate==dirty) ? "*" : "" );
this->SetWindowText(t1);
}
What happens is that the window text gets set to the contents of vi.GetProductName(); everything after that is lost. However, when I change the vi.GetProductName() definition to return a dummy value, it all works correctly.
So, it appears that my member function vi.GetProductName() is doing something weird with the return value.
vi is an instance of the class VersionInformation, which I created. It looks like this:
I seem to remember having a problem with const-ness in this function; I had to make something non-const even though it was const.
#include "stdafx.h"
#include "VersionInformation.h"
CString VersionInformation::get_string(char *buffer, const CString& s)
{
void *pb2;
unsigned int bs2;
char s2buffer[300+1];
s2buffer[0]=0x00;
CString s2,rv;
s2.Format("\\StringFileInfo\\%04x%04x\\%s",langu,codep,s);
if (s2.GetLength()<sizeof(s2buffer))
{
strcpy(s2buffer,s2);
VerQueryValue(buffer,s2buffer,&pb2,&bs2);
}
{
for (unsigned int i=0;i<bs2;i++)
{
rv += ((char*)pb2)[i];
}
}
return rv;
}
VersionInformation::VersionInformation()
{
this->m_filedescription = "";
this->m_productname = "";
this->m_productversion = "";
this->m_copyright = "";
this->m_originalfilename = "";
this->m_internalname = "";
this->m_companyname = "";
this->langu = 0;
this->codep = 0;
}
VersionInformation::~VersionInformation()
{
}
VersionInformation& VersionInformation::operator= (const VersionInformation& vi2)
{
m_filedescription = vi2.m_filedescription;
m_productname = vi2.m_productname;
m_productversion = vi2.m_productversion;
m_copyright = vi2.m_copyright;
m_originalfilename = vi2.m_originalfilename;
m_internalname = vi2.m_internalname;
m_companyname = vi2.m_companyname;
langu = vi2.langu;
codep = vi2.codep;
return *this;
}
void VersionInformation::Load()
{
CString s2;
DWORD dw; // handle, apparently unused.
void *pb2;
unsigned int bs2;
char *buffer = NULL;
{
char mfn[MAX_PATH];
int rs;
if ( GetModuleFileName(NULL,mfn,sizeof(mfn)) == 0 ) goto fail;
mfn[MAX_PATH-1] = 0x00;
if ( 0 == (rs=::GetFileVersionInfoSize(mfn,&dw)) ) goto fail;
if ( NULL == (buffer=(char*)malloc(rs)) ) goto fail;
if ( 0 == ::GetFileVersionInfo(mfn,dw,rs,buffer)) goto fail;
}
if ( 0 == VerQueryValue(buffer,"\\VarFileInfo\\Translation",&pb2,&bs2) ) goto fail;
if ( bs2 < sizeof(DWORD) ) goto fail;
langu = ((WORD*)pb2)[0];
codep = ((WORD*)pb2)[1];
this->m_filedescription = this->get_string(buffer,"FileDescription");
this->m_productname = this->get_string(buffer,"ProductName");
this->m_productversion = this->get_string(buffer,"ProductVersion");
this->m_copyright = this->get_string(buffer,"LegalCopyright");
this->m_originalfilename = this->get_string(buffer,"OriginalFilename");
this->m_internalname = this->get_string(buffer,"InternalName");
this->m_companyname = this->get_string(buffer,"CompanyName");
fail:
free(buffer);
return;
}
CString VersionInformation::GetFileDescription() { return m_filedescription; }
CString VersionInformation::GetProductName() { return CString("XXX"); /*return m_productname;*/ }
CString VersionInformation::GetProductVersion() { return m_productversion; }
CString VersionInformation::GetCopyright() { return m_copyright; }
CString VersionInformation::GetOriginalFilename() { return m_originalfilename; }
CString VersionInformation::GetInternalName() { return m_internalname; }
CString VersionInformation::GetCompanyName() { return m_companyname; }
>>>> t1 = "(DEBUG) "+t1;
The problem is probably here. Look at the right side of the assignment.
The left argument for the + operation is a const char* pointer. The right side is a CString which has a implicit cast operator to a const char* as well. Hence, the operator + adds some pointers and the result is a pointer as well but pointing somewhere into the (virtual) memory.
What you need is
t1 = CString("(DEBUG) ") + t1;
Now the left operand is a CString and the compiler knows to take the CString::operator+ instead of adding pointers.
The problem is probably here. Look at the right side of the assignment.
The left argument for the + operation is a const char* pointer. The right side is a CString which has a implicit cast operator to a const char* as well. Hence, the operator + adds some pointers and the result is a pointer as well but pointing somewhere into the (virtual) memory.
What you need is
t1 = CString("(DEBUG) ") + t1;
Now the left operand is a CString and the compiler knows to take the CString::operator+ instead of adding pointers.
The problem is actually an interesting property of how CString works, combined with a misunderstanding with how VerQueryValue() works.
First, VerQueryValue() - it is returning a buffer size that includes the NULL terminator, not the size that strlen() would return on the buffer - it is one more.
You have code to copy the buffer to a CString one character at a time in your get_string() function:
for (unsigned int i=0;i<bs2;i++)
{
rv += ((char*)pb2)[i];
}
The problem is that bs2 contains the size of the buffer (including the NULL), not the number of characters.
So when you get to last character, you are essentially doing
rv += '\0';
What many people don't know is that CStrings happily work with binary data. And they keep track of their length on their own (that is, they don't use a strlen() equivalent to return the length). So when you append a NULL, you are actually embedding a NULL into your string data.
All of your other concatenatations are working correctly, but because the CString has an embedded NULL, only the part in front of the NULL gets displayed. If you look at the CString in a memory window you will see all of the characters there.
Solution:
Instead of looping through the characters and appending one by one, just assign the buffer:
rv = (char *)pb2;
Or go through your loop one less time:
for (unsigned int i=0; i<bs2-1; i++) // <---- bs2 - 1
{
rv += ((char*)pb2)[i];
}
Then you don't get the embedded NULL, and everything works as expected.
First, VerQueryValue() - it is returning a buffer size that includes the NULL terminator, not the size that strlen() would return on the buffer - it is one more.
You have code to copy the buffer to a CString one character at a time in your get_string() function:
for (unsigned int i=0;i<bs2;i++)
{
rv += ((char*)pb2)[i];
}
The problem is that bs2 contains the size of the buffer (including the NULL), not the number of characters.
So when you get to last character, you are essentially doing
rv += '\0';
What many people don't know is that CStrings happily work with binary data. And they keep track of their length on their own (that is, they don't use a strlen() equivalent to return the length). So when you append a NULL, you are actually embedding a NULL into your string data.
All of your other concatenatations are working correctly, but because the CString has an embedded NULL, only the part in front of the NULL gets displayed. If you look at the CString in a memory window you will see all of the characters there.
Solution:
Instead of looping through the characters and appending one by one, just assign the buffer:
rv = (char *)pb2;
Or go through your loop one less time:
for (unsigned int i=0; i<bs2-1; i++) // <---- bs2 - 1
{
rv += ((char*)pb2)[i];
}
Then you don't get the embedded NULL, and everything works as expected.
> The left argument for the + operation is a const char* pointer. The right side is a CString which has a implicit cast operator to a const char* as well. Hence, the operator + adds some pointers and the result is a pointer as well but pointing somewhere into the (virtual) memory.
This is incorrect, there are multiple friend operator+ functions defined that allow CString's and pointers to be used in either order.
This is incorrect, there are multiple friend operator+ functions defined that allow CString's and pointers to be used in either order.
>>>> vi.GetProductName() + " - " + GetDocumentTitleBaseName()
What is the declaration of the function GetDocumentTitleBaseName?
If it returns a const char* or a LPCSTR (what is a const char* as well) then you were adding two pointers. You only can concatenate using the + operator if the left argument of any + operation is a CString.
What is the declaration of the function GetDocumentTitleBaseName?
If it returns a const char* or a LPCSTR (what is a const char* as well) then you were adding two pointers. You only can concatenate using the + operator if the left argument of any + operation is a CString.
>>>> This is incorrect, there are multiple friend operator+ functions
I know that I added such an operator to my private string class. I don't know for sure that CString has such an operator which allows to add a left hand char pointer. I mean to remember that I recently had problems with that but I may be wrong.
I know that I added such an operator to my private string class. I don't know for sure that CString has such an operator which allows to add a left hand char pointer. I mean to remember that I recently had problems with that but I may be wrong.
> What is the declaration of the function GetDocumentTitleBaseName?
Doesn't matter because vi.GetProductName() is returning a CString, you can have as many pointers to the right of the CString as you want, they are processed from left to right one at a time, with a CString as the result of each intermediate step. Walk through it in the debugger if you don't believe me.
Anyway, as I posted above the problem has nothing to do with faulty pointer arithmetic.
Doesn't matter because vi.GetProductName() is returning a CString, you can have as many pointers to the right of the CString as you want, they are processed from left to right one at a time, with a CString as the result of each intermediate step. Walk through it in the debugger if you don't believe me.
Anyway, as I posted above the problem has nothing to do with faulty pointer arithmetic.
> I don't know for sure that CString has such an operator which allows to add a left hand char pointer.
It's shown in the 6.0 CString MSDN pages, and I was able to walk though it in the debugger in VS2005 and see the functions.
It's shown in the 6.0 CString MSDN pages, and I was able to walk though it in the debugger in VS2005 and see the functions.
Thank you for pointing out my obvious error. I did know that CStrings maintained their own length and could work with binary data but, combined with my misunderstanding about VerQueryString(), my brain didn't consider it !
>>>> I seem to remember having a problem with
>>>> const-ness in this function; I had to make something non-const even though it was const.
I had no problem to compile after setting the member function to const.
>>>> const-ness in this function; I had to make something non-const even though it was const.
I had no problem to compile after setting the member function to const.
// class definition only to make it compile
class VersionInformation
{
public:
CString get_string(char *buffer, const CString& s) const;
private:
DWORD langu;
DWORD codep;
char s[32];
};
CString VersionInformation::get_string(char *buffer, const CString& s) const
{
void *pb2;
unsigned int bs2;
char s2buffer[300+1];
s2buffer[0]=0x00;
CString s2,rv;
s2.Format("\\StringFileInfo\\%04x%04x\\%s",langu,codep,s);
if (s2.GetLength()<sizeof(s2buffer))
{
strcpy(s2buffer,s2);
VerQueryValue(buffer,s2buffer,&pb2,&bs2);
}
{
for (unsigned int i=0;i<bs2;i++)
{
rv += ((char*)pb2)[i];
}
}
return rv;
}
Featured Post
We value your feedback.
Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!
Question has a verified solution.
If you are experiencing a similar issue, please ask a related question
Introduction:
Load and Save to file, Document-View interaction inside the SDI.
Continuing from the second article about sudoku.
Open the project in visual studio.
From the class view select CSudokuDoc and double click to open the header …
Introduction:
Dialogs (1) modal - maintaining the database.
Continuing from the ninth article about sudoku.
You might have heard of modal and modeless dialogs. Here with this Sudoku application will we use one of each type: a modal dialog …
This tutorial covers a step-by-step guide to install VisualVM launcher in eclipse.
The viewer will learn how to use and create keystrokes in Netbeans IDE 8.0 for Windows.
Suggested Courses
Course of the Month8 days, 13 hours left to enroll
734 members asked questions and received personalized solutions in the past 7 days.
Join the community of 500,000 technology professionals and ask your questions.