|
The format method does not properly deal with long output strings. Changing _vstprintf to _vsntprintf and passing in the current size fixes the problem. A obvious oversight given the code logic supports arbitrary output sizes, but it certainly confirms the need to test completely before use...
|
|
|
|
|
If you do decide to use exception handling, please use Win32 SEH. Win32 SEH is platform independent and is the only exception handling mechanism supported by Windows CE.
|
|
|
|
|
Unfortunately, SEH does not correctly call destructors during the stack unwind. So, if you're using smart pointers in a SEH guarded block, you will have a resource leak.
|
|
|
|
|
Currently under CE we have not experienced any problems with SEH. In fact, it has worked remarkably well and fits will into our architecture which is embedded in nature.
Demir Ateser
Intermec Technologies Corp.
|
|
|
|
|
Well, I looked into it a little more closely and you are right. Even under CE the problem does rear its ugly head.
Solution:
Do not construct within a guarded block. If so, then call the destructor explicitly.
|
|
|
|
|
Can someone suggest an extension to TrimLeft() and TrimRight() that takes a string parameter containing the char's to be trimmed?
Thanks
|
|
|
|
|
A version to Trim char(s) of your choice has been added to the VC++ Library
WR
|
|
|
|
|
If you try to construct a CCOMString with an empty BSTR I get a violation. The _tcslen(OLE2T( call is failing if the BSTR is of 0 length. example
CComBSTR b(L"");
CCOMString s(b); <-- will cras
|
|
|
|
|
I solved the above by doing the following.
CCOMString::CCOMString(BSTR bstrString)
{
USES_CONVERSION;
m_pszString = NULL;
if (SysStringLen(bstrString) > 0)
{
int nLen = _tcslen(OLE2T(bstrString));
AllocString(nLen);
_tcsncpy(m_pszString, OLE2T(bstrString), nLen);
}
else
_tcsncpy(m_pszString, _T(""), 0);
}
Apparently this is a problem with all the constructors when the in parameter is empty. I found the same problem when passing an LPCTSTR
|
|
|
|
|
I fixed the LPCTSTR constructor with the following
CCOMString::CCOMString(LPCTSTR pszString)
{
m_pszString = NULL;
if (lstrlen(pszString) > 0)
{
int nLen = _tcslen(pszString);
AllocString(nLen);
_tcsncpy(m_pszString, pszString, nLen);
}
else
_tcsncpy(m_pszString, _T(""), 0);
}
|
|
|
|
|
Hi
One thing that I am having problems with is that if I have arguments to the Format function that are CCOMString's I have to explicitly cast them LPCTSTR or else I get all kinda weird problems. Is there way for this cast to be made implicit. Great job by the way this is very handy for the work I am doing.
|
|
|
|
|
Hi,
If you want to be able to pass a CCOMString as a format argument, all you need to do is change the CCOMString destructor to be non-virtual. Try it and you'll see it works.
What this will do is change the binary layout of the class so that the first 4 bytes of it are the actual string pointer m_pszString instead of the VPTR. This will allow you to pass it to Format (or any other variadic function) as an argument without casting.
What this will take away from you, is the ability to safely derive classes from CCOMString. That's probably not an issue for you. MFC's CString class has a non-virtual destructor for just this purpose (i.e. preserving the simple binary layout)
Joe O'Leary
|
|
|
|
|
These functions are very useful
under certain cases.
|
|
|
|
|
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.
|
|
|
|
|