Link to home
Start Free TrialLog in
Avatar of muttley3141
muttley3141

asked on

MFC CString concatenation failure

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 ?
Avatar of wayside
wayside

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);
}

Open in new window

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.  
Avatar of muttley3141

ASKER

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.
#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; }

Open in new window

>>>>  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.


ASKER CERTIFIED SOLUTION
Avatar of wayside
wayside

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
> 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.
>>>> 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.
>>>> 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.
> 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.
> 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.
>>>>  rv += '\0';

Nice catch ;-)
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.

// 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;
}

Open in new window