|
Paul,
Thank you very much for your contribution of such a thoroughly implemented CString class to the ATL community.
Thank you thank you thank you...
Brian Har
|
|
|
|
|
Paul, Well done, indeed!
|
|
|
|
|
You need to initialize the pointer m_pszString in the standard constructor like show below to avoid access violations.
CCOMString::CCOMString()
{
m_pszString = NULL; // add this line to solve the problem
AllocString(0);
}
|
|
|
|
|
I realize this isn't really part of the CCOMString class, but can anyone tell me how to do an atol on these things?
i'm trying to parse strings with this form: "343:343:22:11".
i need to find the colons, extract the string and convert it to a long.
i tried wcstol and atol, like this:
m_lCat = wcstol(t.GetString(), &stoppedAt, 10);
but the compiler hates this and tells me:
"error C2664: 'wcstol' : cannot convert parameter 1 from 'char *' to 'const unsigned short *' Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast"
any ideas
|
|
|
|
|
The fix is actually pretty simple. The problem comes from the use of TCHAR as the underlying data type. wcstol was designed to allways use wide strings (WCHAR). You need to use the generic string _tcstol function which will use the appropriate wide or ansi string depending on the _UNICODE/_MCBS flags.
m_lCat = _tcstol(t.GetString(), &stoppedAt, 10);
Most of the CRT functions have _tcs type equivelents. Check the MSDN library on DVD if you got it or msdn.microsoft.com for the latest and greatest.
|
|
|
|
|
Down load source missing this line.
m_pszString = NULL;
so full default constructor is
CCOMString::CCOMString()
{
m_pszString = NULL;
AllocString(0);
}
missing above line raise heap error at program running
|
|
|
|
|
This has been fixed. Thank you!
|
|
|
|
|
I noticed a bug in the Format function (ARGH!!! This took me a while to track down...
Anyways, the format call (witch look like this)
void CCOMString::Format(LPCTSTR pszCharSet, ...)
{
va_list vl;
va_start(vl, pszCharSet);
TCHAR* pszTemp = NULL;
int nBufferSize = 0;
int nRetVal = -1;
do {
nBufferSize += 100;
delete [] pszTemp;
pszTemp = new TCHAR[nBufferSize];
////////////////////////////////////
// The problem is the line below!!
nRetVal = _vstprintf(pszTemp, pszCharSet, vl);
} while (nRetVal < 0);
int nNewLen = _tcslen(pszTemp);
AllocString(nNewLen);
_tcscpy(m_pszString, pszTemp);
delete [] pszTemp;
va_end(vl);
}
As you can see (and I now know) is that the vstprintf is as usualy quite stupid and will overwrite the buffer if the buffer is to small, (witch it will be if the string is over 100 chars the first pass, but ofcourse the vstprintf will not notice this an happily say it all worked fine...)
So, to make this work change the "faulty" line to this one:
nRetVal = _vsnprintf(pszTemp, nBufferSize, pszCharSet, vl);
witch will take the current buffer size as an argument and thus will not bufferoverflow your code... (I hope
The function as a whole: (nicly formated for your convinience (damn thats hard one to spell):
void CCOMString::Format(LPCTSTR pszCharSet, ...)
{
va_list vl;
va_start(vl, pszCharSet);
TCHAR* pszTemp = NULL;
int nBufferSize = 0;
int nRetVal = -1;
do {
nBufferSize += 100;
delete [] pszTemp;
pszTemp = new TCHAR[nBufferSize];
nRetVal = _vsnprintf(pszTemp, nBufferSize, pszCharSet, vl);
} while (nRetVal < 0);
int nNewLen = _tcslen(pszTemp);
AllocString(nNewLen);
_tcscpy(m_pszString, pszTemp);
delete [] pszTemp;
va_end(vl);
}
o/w on the whole, I use this class all over, and I tend to love it
The only thing I would like is to make it cast to a LPCTSTR "default" to be able to ATLTRACE() it without having to cast it first, if anyone know how, PLEASE let me know
// Micke
|
|
|
|
|
On second thought... (it is not the buffersize you pass along it is actuly the maximum number of chars to write, so you better decrease that one abit to make room for the trailing null)
void CCOMString::Format(LPCTSTR pszCharSet, ...)
{
va_list vl;
va_start(vl, pszCharSet);
TCHAR* pszTemp = NULL;
int nBufferSize = 0;
int nRetVal = -1;
do {
nBufferSize += 100;
delete [] pszTemp;
pszTemp = new TCHAR[nBufferSize];
nRetVal = _vsnprintf(pszTemp, nBufferSize-1, pszCharSet, vl);
} while (nRetVal < 0);
int nNewLen = _tcslen(pszTemp);
AllocString(nNewLen);
_tcscpy(m_pszString, pszTemp);
delete [] pszTemp;
va_end(vl);
}
|
|
|
|
|
The source code has been changed to reflect the above code. Thanks very much for your suggestion.
Pau
|
|
|
|
|
Hey Paul =)
You must not have tested this with a UNICODE build. Try it, check the output, then look at your calls to ATLTRACE. Notice something missing? (Like maybe the _T( and )?) =)
No complaints, though I would recommend using new and delete rather than malloc/free. Arguments can be made that realloc() is just too easy to use, so malloc/free is a small price to pay. In my opinion, using new/delete is just better C++ practice. Probably the nicest thing about using new is that you don't need to worry about sizeof(TCHAR) when allocating memory. Makes things alot easier to read. Also, it keeps things consistent with the rest of the C++ code that will be using the string class. My $.02.
One other thing: You might want to consider relieving yourself of the dependency on atlbase.h, since there's no guarantee users will have (or want to use) ATL. With a few modifications, this would be an easy cross-platform string class. You might add some #if defined(_WIN32)'s in there for the BSTR stuff, and use MultiByteToWideChar and WideCharToMultiByte for BSTR conversions. It's basically the same thing that is going on under the hood with the X2Y macros, anyhow. The only real difference is that memory allocated by the macros is done on the stack (via alloca) rather than on the heap. Okay, make that $.04. =)
Damn, make that $.06: One method of memory tracking that might make things a bit faster (though more costly with memory) is to allocate the string storage by blocks, rather than by _tcslen(). For 0 size allocation, you could simply use a static LPCTSTR method that returns _T("") and assign it to the m_pszString member and set an allocation size member to 0. This would also greatly improve performance in the case where strings grow to huge sizes. Since you are tracking memory allocation, you could easily track current string length, so GetLength() could return an int member rather than calling _tcslen() each time. Something to chew on.
Again, I'm not at all complaining, I think you did a great job. =) I just like to add suggestions if I have something constructive to say. Please don't take that the wrong way. I do have to say: It's nice to have an alternative to basic_string<>. And since I rarely use MFC (absolutely NEVER for COM), it's easy to drop something like this in. Yeah, there's WTL's CString, but not everyone has WTL, and trying to develop in a group of engineers and having to make everyone set up WTL paths is a huge pain. =)
Thanks again, Paul!
P.S. I plan on adding the things mentioned above to your class. If you'd like me to send you the result, please email me. Time permitting, I should be able to get to it in the next few days.
|
|
|
|
|
Hey Paul =)
You must not have tested this with a UNICODE build. Try it, check the output, then look at your calls to ATLTRACE. Notice something missing? (Like maybe the _T( and )?) =)
No complaints, though I would recommend using new and delete rather than malloc/free. Arguments can be made that realloc() is just too easy to use, so malloc/free is a small price to pay. In my opinion, using new/delete is just better C++ practice. Probably the nicest thing about using new is that you don't need to worry about sizeof(TCHAR) when allocating memory. Makes things alot easier to read. Also, it keeps things consistent with the rest of the C++ code that will be using the string class. My $.02.
One other thing: You might want to consider relieving yourself of the dependency on atlbase.h, since there's no guarantee users will have (or want to use) ATL. With a few modifications, this would be an easy cross-platform string class. You might add some #if defined(_WIN32)'s in there for the BSTR stuff, and use MultiByteToWideChar and WideCharToMultiByte for BSTR conversions. It's basically the same thing that is going on under the hood with the X2Y macros, anyhow. The only real difference is that memory allocated by the macros is done on the stack (via alloca) rather than on the heap. Okay, make that $.04. =)
Damn, make that $.06: One method of memory tracking that might make things a bit faster (though more costly with memory) is to allocate the string storage by blocks, rather than by _tcslen(). For 0 size allocation, you could simply use a static LPCTSTR method that returns _T("") and assign it to the m_pszString member and set an allocation size member to 0. This would also greatly improve performance in the case where strings grow to huge sizes. Since you are tracking memory allocation, you could easily track current string length, so GetLength() could return an int member rather than calling _tcslen() each time. Something to chew on.
Again, I'm not at all complaining, I think you did a great job. =) I just like to add suggestions if I have something constructive to say. Please don't take that the wrong way. I do have to say: It's nice to have an alternative to basic_string<>. And since I rarely use MFC (absolutely NEVER for COM), it's easy to drop something like this in. Yeah, there's WTL's CString, but not everyone has WTL, and trying to develop in a group of engineers and having to make everyone set up WTL paths is a huge pain. =)
Thanks again, Paul!
P.S. I plan on adding the things mentioned above to your class. If you'd like me to send you the result, please email me. Time permitting, I should be able to get to it in the next few days.
|
|
|
|
|
I have been trying out your CCOMString class in a couple of my ATL projects... It appeared to be working great until I noticed my programs were leaking memory, and the amount of memory leaked was dependent on the size of certain input strings to my program.
It seems that your CCOMString leaks when I do something like this:
SomeFunc(BSTR strInput)
{
CCOMString strTest(strInput);
}
If the strInput is big enough you'll begin to notice the memory leaks (I watched my program in NT task manager to see the leaks). I only tested this problem in an ATL project so I couldn't use debug_new to have Visual C report the leaks to me. I replaced all my CCOMString's with WTL::CString's and the memory leaking went away.
I'm not sure if other types of strings other than BSTR's cause your string class to leak, I'm guessing probably not. Just thought you would want to know about this
|
|
|
|
|
I also had problems with this, and emailed the author (he said hed update the article) but hers a simple fix:
*ALL* constructors need to set m_pszString to NULL:
CCOMString::CCOMString()
{
m_pszString = NULL;
AllocString(0);
}
The destructor should look loke this:
CCOMString::~CCOMString()
{
free(m_pszString);
}
And finaly the AllocString:
void CCOMString::AllocString(int nLen)
{
if (m_pszString != NULL)
free(m_pszString);
ATLASSERT(nLen >= 0);
m_pszString = (TCHAR*) malloc((nLen+1) * sizeof(TCHAR));
ATLASSERT(m_pszString != NULL);
m_pszString[nLen] = '\0';
}
(I think that was all I changed, dont recall, but generaly the idea is to make sure the m_pszString is destroyed everywhere it either is realloced or destroyed)
I also added a few more functins to make it more like CString, but thats another story...
|
|
|
|
|
An update to the source code was sent to Code Project on June 28, 2000. If you need a copy of the update, please email me and I will provide a copy to you.
Thanks,
Pau
|
|
|
|
|
FYI, Dr. Richard Grimes wrote an article about the WTL in the April, 2000 issue of Visual C++ Developer's Journal if anyone's interested. Unfortunately, that article was the first time I had ever heard of the WTL. Nonetheless, writing CCOMString was a great learning experience
|
|
|
|
|
The good old CString is there along with MFC-like collections
|
|
|
|
|