Playing with CString and LPSTR, PLEASE HELP !!

[Braulio]

Hi,

I'm trying to do a "replace" function that goes fast, my replace function is:

ReplaceString(CString &Text,int iBegin,int iEnd,CString &NewString)

And puts the "NewString" in the string "Text", deleting what is between iBegin and iEnd, for example:

Text = "aabbcccc"
NewString = "d"
ReplaceString(Text,2,3,NewString)

Make:
Text = "aadccc"

The problem is the case:

Text = "aabbc"
NewText = "defg"
ReplaceString(Text,3,4,NewString)

Gives me an error when tries to destroy the CString !, Here is the piece of code that I use for that case:

iResize = iNewSubStringLength-iOldSubStringLength;
pText = Text.GetBuffer(iTextLength+iResize); // +1?
pNewString = NewString.GetBuffer(0);

int iDst = iBegin+iOldSubStringLength;
int iSrc = iDst + iResize;
memmove(pText+iSrc,pText+iDst,sizeof(_T(' '))*iResize);
_tcsncpy(pText+iBegin,pNewString,iNewSubStringLength);
//memcpy(pText+iBegin,pNewString,iNewSubStringLength);
Text.ReleaseBuffer(iTextLength+iResize);

[LALeonard]

For one thing, that GetBuffer(0) is wrong. You're telling it to give you a pointer to zero chars! That zero should be the largest size you are going to need for that variable.

<HR>LA Leonard - <A HREF="http://www.DefinitiveSolutions.com/">Definitive Solutions, Inc.</A>

I have used this method to obtain the buffer to a CString of known size: replace
pNewString = NewString.GetBuffer(0);
with
pNewString = NewString.GetBuffer( NewString.GetLength() );

[Jason Teagle]

I think the other replies have shown the problem - the GetBuffer(0). Although this appears to work, because the pointer points to 0 bytes (usually, although the allocation COULD be larger), you are copying beyond that; hence, you are writing into memory that you are unlikely to own. When the CString is deconstructed, this is when the memory system fouls up.

However, I have to ask, is this method really too slow for what you want?:

---
void ReplaceString(CString& strText, int iBegin, int iEnd, CString strReplace)
{
// iBegin and iEnd are 1-based.

CString strRightSide ;
int iLength ;

iLength = strText.GetLength();
strRightSide = strText.Right(iLength - iEnd);
strText = strText.Left(iBegin) + strReplace + strRightSide ;
}
---
(note that the second CString is not passed by reference - because you are not going to change it, it doesn't need to be).

It's certainly safer, and easier to read and understand; but perhaps the speed is critical.

[Braulio]

Hi !,

Finally I have made my function work ( I think the GetBuffer(0) is not so problem, if you don't have to make the buffer bigger, at least is what I have seen debugging, altough I'm not very good doing that things...).

The problem is that is still to Slow !, agh !, all the day thinking and thinking, and very slow... ( I compare it with an extension of CString that Zafir Anjum has made one the CString, that goes as the demon, but my boss don't want that I inherit from CString a new class :-().

I have to make the replace with very big files with a lot of things to replace, here goes what I have done finally:

/* -------------------------------------------------------------------------
Replace the text given betwen iBeginRep and iEndReplace, with
NewString
------------------------------------------------------------------------- */
void CReplace::ReplaceString(CString &Text,int iBegin,int iEnd,CString &NewString)
{
LPTSTR pText = NULL;

if (iEnd - iBegin == NewString.GetLength()) { // Only Replace
pText = Text.GetBuffer(0);
_tcsncpy(pText+iBegin, NewString, iEnd - iBegin);
Text.ReleaseBuffer();
} else {
if (iEnd - iBegin > NewString.GetLength()) { // Replace and Move to right
pText = Text.GetBuffer(0);
_tcsncpy(pText+iBegin, NewString, NewString.GetLength());
//memmove(pText+iBegin+NewString.GetLength(), pText+iEnd, (Text.GetLength() - iEnd) * sizeof(TCHAR));
memmove(pText+iBegin+NewString.GetLength(),pText+iEnd,
(Text.GetLength()-(iBegin+NewString.GetLength()))* sizeof(TCHAR));
Text.ReleaseBuffer(-1);
} else { // Make Buffer Bigger,Move to left and Replace
int iMore = NewString.GetLength() - (iEnd-iBegin);
pText = Text.GetBufferSetLength(Text.GetLength() + iMore);
//memmove(pText+iEnd+iMore, pText+iEnd, (Text.GetLength() - iEnd + iMore) * sizeof(TCHAR));
memmove(pText+iEnd+iMore // -iMore Because The String has been made "iMore" Bigger !!!!!!!!!!
, pText+iEnd,
(Text.GetLength() - iEnd - iMore) * sizeof(TCHAR));
_tcsncpy(pText+iBegin, NewString, NewString.GetLength());
Text.ReleaseBuffer(-1);
}
}
}

I don't now if I could be able to make it faster..., Thanks, Bye !
Braulio

[Braulio]

Hi !,

Finally I have made my function work ( I think the GetBuffer(0) is not so problem, if you don't have to make the buffer bigger, at least is what I have seen debugging, altough I'm not very good doing that things...:-( ).

The problem is that is still to Slow !, agh !, all the day thinking and thinking, and very slow... ( I compare it with an extension of CString that Zafir Anjum has made one the CString, that goes as the demon, but my boss don't want that I inherit from CString a new class :-().

I have to make the replace with very big files with a lot of things to replace, here goes what I have done finally:

/* -------------------------------------------------------------------------
Replace the text given betwen iBeginRep and iEndReplace, with
NewString
------------------------------------------------------------------------- */
void CReplace::ReplaceString(CString &Text,int iBegin,int iEnd,CString &NewString)
{
LPTSTR pText = NULL;

if (iEnd - iBegin == NewString.GetLength()) { // Only Replace
pText = Text.GetBuffer(0);
_tcsncpy(pText+iBegin, NewString, iEnd - iBegin);
Text.ReleaseBuffer();
} else {
if (iEnd - iBegin > NewString.GetLength()) { // Replace and Move to right
pText = Text.GetBuffer(0);
_tcsncpy(pText+iBegin, NewString, NewString.GetLength());
//memmove(pText+iBegin+NewString.GetLength(), pText+iEnd, (Text.GetLength() - iEnd) * sizeof(TCHAR));
memmove(pText+iBegin+NewString.GetLength(),pText+iEnd,
(Text.GetLength()-(iBegin+NewString.GetLength()))* sizeof(TCHAR));
Text.ReleaseBuffer(-1);
} else { // Make Buffer Bigger,Move to left and Replace
int iMore = NewString.GetLength() - (iEnd-iBegin);
pText = Text.GetBufferSetLength(Text.GetLength() + iMore);
//memmove(pText+iEnd+iMore, pText+iEnd, (Text.GetLength() - iEnd + iMore) * sizeof(TCHAR));
memmove(pText+iEnd+iMore // -iMore Because The String has been made "iMore" Bigger !!!!!!!!!!
, pText+iEnd,
(Text.GetLength() - iEnd - iMore) * sizeof(TCHAR));
_tcsncpy(pText+iBegin, NewString, NewString.GetLength());
Text.ReleaseBuffer(-1);
}
}
}

I don't now if I could be able to make it faster..., Thanks, Bye !
Braulio

[Jason Teagle]

I was obviously asleep when I responded earlier. Now I realise that the GetBuffer(0) does NOT matter, because you are copying FROM that string, not TO it - so all you need is a pointer to it.

This actually means that you don't need GetBuffer() at all - you can simply say

pText = (const char *)Text ;

This should be marginally faster since the MFC doesn't need to allocate a buffer for it, but simply provides a pointer to the string of characters.

[Braulio]

Hi again !,

If I get a const pointer, then can I modify on it ?, I use GetBuffer because gives me a LPSTR pointer not a constant pointer, and the I can modify on it, may can I do the same with "pText = (const char *)Text ;" ?

Thanks, Bye
Braulio

PS.: I never thought that dealing with string were so complicated until now... ;-)

[Dave Lorde]

It's not necessarily wrong.

GetBuffer(0) will get a pointer to a buffer with a *minimum* length of 0. So it return a pointer to a buffer the length of the string in CString.

Dave

[Dave Lorde]

No, you can't modify the buffer via a const char* without casting, and it's dangerous to cast away the const for pointers to CString buffers, because the buffer may be shared between several CStrings!

This is why GetBuffer is provided; it copies the buffer if it's shared or if it's shorter than you requested.

Dave

[Braulio]

Hi,

The problem was the ReleaseBuffer, I pass as Length -1, and looked for end character every time, now I put the length, and it goes a little bit faster than Zafir's one, YUUUUJUUUUU !!!!

Thanks for your help, if you want the code, please say it to me, and I will send it to you, Bye !
Braulio

[Jason Teagle]

No, you can't modify the const pointer - but that is the whole point. Your routine doesn't need to modify the string supplied as a replacement, does it? It is only copying from it, which is fine with a const pointer.

[Jason Teagle]

Every time I read your code snippet I seem to see something different (maybe I'm getting too old for this and my eyesight is going {:v) ). Here are the CString rules:

1. If you want to modify a string, you MUST use GetBuffer() to access it.
2. When you use GetBuffer(), you MUST specify the maximum length you expect the string to be - if you modify beyond this, you will most likely get a crash. Thus, when calling GetBuffer() for the string which will have the replacement put in it, you CANNOT use 0 as the parameter. This (as I see now) corresponds to Text in your routine.
3. The string you are using to replace from is not going to be modified - therefore it can use the (const char *)string cast to access the string. This (as I see now) corresponds to NewString in your routine.

Sorry for the confusion.

[Jason Teagle]

Yes, but if 0 is a minimum then it COULD be a buffer of 0 length, couldn't it? Minimum doesn't necessarily mean it will always allocate larger, and you should certainly NOT rely on the MFC allocating as much space as there are characters in the string - unless you have been through the MFC code and verified that this is true. The help does not specify how large the buffer will be.

[Dave Lorde]

> Yes, but if 0 is a minimum then it COULD be a buffer of 0 length, couldn't it?

It depends what you mean by a buffer of 0 length :-)

Even when the CString is empty and the allocated length is zero, the buffer pointer points to a static null character, so GetBuffer(0) will return a pointer to a zero-terminated buffer of 1 character (i.e. a character string of length 0).

> you should certainly NOT rely on the MFC allocating as much space as there are characters in the string

No. GetBuffer returns a pointer to the buffer for you to play with. It allows you to set a minimum length for the buffer, meaning that if the string is shorter than this, it will allocate extra space to make it up to the specified minimum.

> unless you have been through the MFC code and verified that this is true

Indeed, I'd have been foolish not to check it first. It only takes a few seconds ;-)

> The help does not specify how large the buffer will be.

Yes, it's not clear really. Always check the source (if available) when in doubt, documentation can also be wrong...

Dave

[Jason Teagle]

Well, I stand corrected. I never knew this. I always assumed I was supposed to supply the minimum size I expected the resulting string to be, so I supplied the maximum size expected to be safe. I guess _I_ should have examined the MFC; but then who wants nightmares?

The point is, in Braulio's example, as the resulting string may have been longer than the original the parameter to GetBuffer() should have been the maximum size expected, which I think (s)he has now done.

[Dave Lorde]

> I guess _I_ should have examined the MFC; but then who wants nightmares?

:-) Actually, once you learn to skip the ASSERTs and conditional compilation stuff, most of the MFC code isn't too bad. It's the Windows framework stuff that's really complex...

> The point is, in Braulio's example, as the resulting string may have been longer than the original the parameter to GetBuffer()
> should have been the maximum size expected, which I think (s)he has now done.

Yes, that's right.

Dave