Stuck up with NULL character
-
Hi, I have written a class as follows: class A { char str[2]; public: A (LPSTR Astr) { strcpy(str, Astr); } A& operator = (LPSTR Astr) { strcpy(str, Astr); return *this; } }; I have created a variable of the class and trying to assign it some values using following code A a1("aa"); and returning this variable from a function. This is failing with the following error message "Run_Time Check Failure#2 - Stack around the variable 'a1' was corrupted" when I do this in a MFC application. This is right since I am not leaving any space for the null character. However when I am doing the same thing in the DLL, it is running perfectly. I am not getting why it is running in DLL and failing in direct Application? Can anybody please explain?
Well, basically why bother if obviously broken code apparently works? Anyway probably you have such information because
MFC(at least in debug build) puts additional code to check memory corruption. Your code is, BTW, dangerous in both the environments. :)If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke -
Hi, I have written a class as follows: class A { char str[2]; public: A (LPSTR Astr) { strcpy(str, Astr); } A& operator = (LPSTR Astr) { strcpy(str, Astr); return *this; } }; I have created a variable of the class and trying to assign it some values using following code A a1("aa"); and returning this variable from a function. This is failing with the following error message "Run_Time Check Failure#2 - Stack around the variable 'a1' was corrupted" when I do this in a MFC application. This is right since I am not leaving any space for the null character. However when I am doing the same thing in the DLL, it is running perfectly. I am not getting why it is running in DLL and failing in direct Application? Can anybody please explain?
abhijitr wrote:
char str[2];
This member variable does not have enough room for two characters PLUS the terminating
\0character."Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
-
abhijitr wrote:
char str[2];
This member variable does not have enough room for two characters PLUS the terminating
\0character."Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
Also, the assignment operator fails to insure there will be no buffer overwrite. Simple assignments such as,
A Astring;
Astring = "too long";will cause errors. Changing the size of the buffer won't be enough. The arguments to the assignment operator should be constant; unless you intend to allow it to change its argument?!?
A& operator = (LPCSTR Astr)
{
// reset the buffer
::memset(str, 0, sizeof(str));if (!Astr) return \*this; int len = ::strlen(Astr); if (len <= sizeof(str)) { // too short or just right ::strcpy(str, Astr); } else if (sizeof(str) < len) { // too long // Q: how to handle the terminal nul??? ::strncpy(str, Astr, sizeof(str) - 1 ); // truncate str\[1\] = '\\0'; // terminate } return \*this;}
Should probably check for the degenerate case of an empty string as well.
-
Also, the assignment operator fails to insure there will be no buffer overwrite. Simple assignments such as,
A Astring;
Astring = "too long";will cause errors. Changing the size of the buffer won't be enough. The arguments to the assignment operator should be constant; unless you intend to allow it to change its argument?!?
A& operator = (LPCSTR Astr)
{
// reset the buffer
::memset(str, 0, sizeof(str));if (!Astr) return \*this; int len = ::strlen(Astr); if (len <= sizeof(str)) { // too short or just right ::strcpy(str, Astr); } else if (sizeof(str) < len) { // too long // Q: how to handle the terminal nul??? ::strncpy(str, Astr, sizeof(str) - 1 ); // truncate str\[1\] = '\\0'; // terminate } return \*this;}
Should probably check for the degenerate case of an empty string as well.
This should be between you and the OP.
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
-
This should be between you and the OP.
"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for, in order to get to the job you need to pay for the clothes and the car and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
Why? I don't understand.
-
Also, the assignment operator fails to insure there will be no buffer overwrite. Simple assignments such as,
A Astring;
Astring = "too long";will cause errors. Changing the size of the buffer won't be enough. The arguments to the assignment operator should be constant; unless you intend to allow it to change its argument?!?
A& operator = (LPCSTR Astr)
{
// reset the buffer
::memset(str, 0, sizeof(str));if (!Astr) return \*this; int len = ::strlen(Astr); if (len <= sizeof(str)) { // too short or just right ::strcpy(str, Astr); } else if (sizeof(str) < len) { // too long // Q: how to handle the terminal nul??? ::strncpy(str, Astr, sizeof(str) - 1 ); // truncate str\[1\] = '\\0'; // terminate } return \*this;}
Should probably check for the degenerate case of an empty string as well.
Your code example may confuse the OP... Your second
if()test checks the length of the passed string against the length of thestrbuffer, instead of "length of thestrbuffer - 1", so thestrcpy(...)executed if theif()passes will overwrite one past the end of thestrbuffer when it writes a terminatingNULcharacter. The finalelse if()test copies the source string correctly, but the line after that places the terminatingNULis using a hardcoded index of1, so ifstris longer than 2 characters, you will copy a truncated version of the source string and then truncate it further to one character. Use ofsizeof()to determine the character length of the buffer only works for (1) arrays that are part of the object, or automatically/locally allocated as arrays (i.e. ifstris actually a pointer to an array and not a real array, it will not work as expected); and (2) only in ANSI builds - for UNICODE, the calculations will be off. The copying of the source string can be further simplified to:dwStrBufferLen = /* character length of the
strbuffer */
::strncpy( str, Astr, dwStrBufferLen );
str[ dwStrBufferLen - 1 ] = '\0';This will work regardless of the length of the target string, truncating it if necessary, and will ensure that a terminating
NULis always there. Since::strncpy(...)will not go past the length specified, it is one less place to be concerned with doing a correct off-by-one offset. If it runs out of space before the end of the source string, the following line will write the terminatingNUL. Peace!-=- James
Please rate this message - let me know if I helped or not! * * *If you think it costs a lot to do it right, just wait until you find out how much it costs to do it wrong!
Remember that Professional Driver on Closed Course does not mean your Dumb Ass on a Public Road!
See DeleteFXPFiles -
Your code example may confuse the OP... Your second
if()test checks the length of the passed string against the length of thestrbuffer, instead of "length of thestrbuffer - 1", so thestrcpy(...)executed if theif()passes will overwrite one past the end of thestrbuffer when it writes a terminatingNULcharacter. The finalelse if()test copies the source string correctly, but the line after that places the terminatingNULis using a hardcoded index of1, so ifstris longer than 2 characters, you will copy a truncated version of the source string and then truncate it further to one character. Use ofsizeof()to determine the character length of the buffer only works for (1) arrays that are part of the object, or automatically/locally allocated as arrays (i.e. ifstris actually a pointer to an array and not a real array, it will not work as expected); and (2) only in ANSI builds - for UNICODE, the calculations will be off. The copying of the source string can be further simplified to:dwStrBufferLen = /* character length of the
strbuffer */
::strncpy( str, Astr, dwStrBufferLen );
str[ dwStrBufferLen - 1 ] = '\0';This will work regardless of the length of the target string, truncating it if necessary, and will ensure that a terminating
NULis always there. Since::strncpy(...)will not go past the length specified, it is one less place to be concerned with doing a correct off-by-one offset. If it runs out of space before the end of the source string, the following line will write the terminatingNUL. Peace!-=- James
Please rate this message - let me know if I helped or not! * * *If you think it costs a lot to do it right, just wait until you find out how much it costs to do it wrong!
Remember that Professional Driver on Closed Course does not mean your Dumb Ass on a Public Road!
See DeleteFXPFilesGood catch! Yes, I missed a few things here. Sorry.
-
Hi, I have written a class as follows: class A { char str[2]; public: A (LPSTR Astr) { strcpy(str, Astr); } A& operator = (LPSTR Astr) { strcpy(str, Astr); return *this; } }; I have created a variable of the class and trying to assign it some values using following code A a1("aa"); and returning this variable from a function. This is failing with the following error message "Run_Time Check Failure#2 - Stack around the variable 'a1' was corrupted" when I do this in a MFC application. This is right since I am not leaving any space for the null character. However when I am doing the same thing in the DLL, it is running perfectly. I am not getting why it is running in DLL and failing in direct Application? Can anybody please explain?
Maybe because you are running the application in debug mode, and the dll in release mode!? Either way you are corrupting memory.
AliR. Visual C++ MVP
-
Hi, I have written a class as follows: class A { char str[2]; public: A (LPSTR Astr) { strcpy(str, Astr); } A& operator = (LPSTR Astr) { strcpy(str, Astr); return *this; } }; I have created a variable of the class and trying to assign it some values using following code A a1("aa"); and returning this variable from a function. This is failing with the following error message "Run_Time Check Failure#2 - Stack around the variable 'a1' was corrupted" when I do this in a MFC application. This is right since I am not leaving any space for the null character. However when I am doing the same thing in the DLL, it is running perfectly. I am not getting why it is running in DLL and failing in direct Application? Can anybody please explain?
-
When I add a virtual destructor to this class it is not giving any error. It runs smoothly. However if I add simple destructor it fails.
Hi Guys, I have finally solved the problem. If you check the size of class A it comes to be 8 (with virtual destructor) which we were expecting it to be 06 as we have used 2 bytes for ‘stre’ and 4 bytes for virtual pointer (as we have declared virtual destructor in class A). On investigation I found that size is 8 due to word alignment of structure and union members which is used to increase the performance. Due to this we are getting room for extra two characters which accommodates our null character and saves us from crashing. Thanks for your cooperation.
-
Hi Guys, I have finally solved the problem. If you check the size of class A it comes to be 8 (with virtual destructor) which we were expecting it to be 06 as we have used 2 bytes for ‘stre’ and 4 bytes for virtual pointer (as we have declared virtual destructor in class A). On investigation I found that size is 8 due to word alignment of structure and union members which is used to increase the performance. Due to this we are getting room for extra two characters which accommodates our null character and saves us from crashing. Thanks for your cooperation.
I don't understand why you are not increasing the size of the buffer to 3 so that you can have 2 chars and a null, and use strncpy instead of strcpy. Why are you insisting on overwriting past your buffer?
AliR. Visual C++ MVP
-
I don't understand why you are not increasing the size of the buffer to 3 so that you can have 2 chars and a null, and use strncpy instead of strcpy. Why are you insisting on overwriting past your buffer?
AliR. Visual C++ MVP
