Solved

MFC CString concatenation failure

Posted on 2008-10-14
13
2,657 Views
Last Modified: 2013-12-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 ?
0
Comment
Question by:muttley3141
  • 6
  • 5
  • 2
13 Comments
 
LVL 14

Expert Comment

by:wayside
ID: 22711111
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

0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 22711683
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.  
0
 
LVL 1

Author Comment

by:muttley3141
ID: 22713169
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

0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 22713749
>>>>  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.


0
 
LVL 14

Accepted Solution

by:
wayside earned 80 total points
ID: 22713779
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.
0
 
LVL 14

Expert Comment

by:wayside
ID: 22713835
> 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.
0
Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 22713853
>>>> 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.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 22713910
>>>> 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.
0
 
LVL 14

Expert Comment

by:wayside
ID: 22713913
> 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.
0
 
LVL 14

Expert Comment

by:wayside
ID: 22713953
> 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.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 22713995
>>>>  rv += '\0';

Nice catch ;-)
0
 
LVL 1

Author Closing Comment

by:muttley3141
ID: 31505841
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 !
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 22714210
>>>> 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

0

Featured Post

Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Title # Comments Views Activity
Prevent this page from creating additional dialogs. 3 385
maven archtype selection in eclipse 1 53
Detect file exist or not 3 127
Problem to open Excel file 15 89
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: Displaying information on the statusbar.   Continuing from the third article about sudoku.   Open the project in visual studio. Status bar – let’s display the timestamp there.  We need to get the timestamp from the document s…
The viewer will learn how to synchronize PHP projects with a remote server in NetBeans IDE 8.0 for Windows.
This video will show you how to get GIT to work in Eclipse.   It will walk you through how to install the EGit plugin in eclipse and how to checkout an existing repository.

947 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

24 Experts available now in Live!

Get 1:1 Help Now