C / C++ / MFC

6-Jul-05 20:16

Good Morning everybody,

i'd like to get your help on that too deep problem for my mind for the moment i am reflecting on.

I have a Dialog based MFC project that i am programming on VS7.1.
It's been compiling well, and i can execute it well as far as i enter in a certain state that causes the crash (so, if there is any syntactic/writing mistake on the code exposed, it is only on this post i make an error - as i don't Copy/Paste all the code given here).

i have two main classes, plus some extra types defined outside the classes as they are used by the two classes :

<code>CFactEditorDlg</code>  (the main dialog of the program)
    - FactEditorDlg.h
    - FactEditorDlg.cpp
 
<code>COptionsDlg</code>  (the option dialog of the program, that allow the user to set some display parameters)
    - OptionsDlg.h
    - OptionsDlg.cpp
 
FactEditorTypes.h (sample) :
<font color=blue>typedef enum</font> {
    <font color=green>//...</font>
} TStartingCallCountry;
 
<font color=blue>typedef enum</font> {
    <font color=green>//...</font>
} TEditorLanguage;
 
<font color=blue>typedef unsigned char</font> TDigitsInYears;
 
<font color=blue>typedef unsigned char</font> TDigitsInPhones;
 
<font color=blue>typedef struct</font> {
    TStartingCallCountry _StartingCallCountry;
    TEditorLanguage      _EditorLanguage;
    CString              _FileViewer;
    TDigitsInYears       _NbDigitsInYears;
    TDigitsInPhones      _NbDigitsInPhones;
} TEditorOptions;

TEditorOptions is a type made to transfer the 5 fields it encapsulates from CFactEditorDlg to the COptionsDlg, and to get them back (maybe modified) then.
This is done in the OnSclickOptions button as follow :

<font color=blue>void</font> CFactEditorDlg::OnSClickOptions() {
    <font color=green>// Creates EditorOptions with 5 data members</font>
    TEditorOptions EditorOptions = {
        m_StartingCallCountry,
        m_EditorLanguage,
        m_strFileViewer,
        m_ucNbDigitsInYears,
        m_ucNbDigitsInPhones
    };
    <font color=green>//Passes the datas through an overloaded constructor</font>
    COptionsDlg OptDlg(EditorOptions, m_strEditorVersion, <font color=blue>this</font>);
    <font color=blue>int</font> iResult = OptDlg.DoModal();
    <font color=blue>if</font> (iResult == IDOK) {
        <font color=green>// Validates the changes</font>
        <font color=green>//...</font>
    }
    <font color=blue>else</font> { <font color=green>// IDCANCEL</font>
    }
}

...and here is how the constructor of COptionsDlg treats the TEditorOptions parameter :

COptionsDlg::COptionsDlg(TEditorOptions& EditorOptions,
                         CString         EditorVersion,
                         CWnd*           pParent <font color=green>/* = NULL */</font>)
        : CDialog(COptionsDlg::IDD, pParent),
            m_EditorOptions(EditorOptions)    <font color=green>// m_EditorOptions is defined as a TEditorOptions&</font>
{
    ::strcpy(m_strEditorVersion, EditorVersion);
    <font color=red>m_strEditorVersion = EditorVersion</font>;    <font color=green>// m_strEditorVersion is now a Cstring</font>
    m_strDigitsInYears.Format(<font color=gray>"%d"</font>, m_EditorOptions._NbDigitsInYears);
    m_strDigitsInPhones.Format(<font color=gray>"%d"</font>, m_EditorOptions._NbDigitsInPhones);
}

now, the problem is that the Options dialog modifies well the datas, returns them to the CFactEditorDlg::OnSClickOptions() event handler correctly (i displayed the resulting values with a MessageBox), but just when i get out the OnSClickOptions() function, i get such a weird error :

Run-Time Check Failure #2 - Stack around the variable 'EditorOptions' was corrupted.

this is exactly what i get from the Run-Time, and i really can't understand what it can mean.

if it is of any help, i tried to see what happens in the debugger, and i saw that the local EditorOptions (local to OnSClickOptions()) is staying with the values it got from the option dialog, except the fact that the CString member _FileViewer is leaving its content.
the debbugger gives this :

Before :
    _FileViewer    {0x002fa000 "notepad"}                             ATL::CStringT<char,StrTraitMFC_DLL<char,ATL::ChTraitsCRT<char> > >
 
After :
    _FileViewer    {0x002fa000 "îþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþ"}    ATL::CStringT<char,StrTraitMFC_DLL<char,ATL::ChTraitsCRT<char> > >

if anybody knows, i'm all listening at you guys...

[edit]
i modified the m_strEditorVersion in both DialogBox classes into CString.
The error remains the same...
[/edit]

TOXCCT >>> GEII power
[toxcct][VisualCalc]

Bob Stanneveld6-Jul-05 21:09

6-Jul-05 21:09

toxcct wrote:
Run-Time Check Failure #2 - Stack around the variable 'EditorOptions' was corrupted.

This means that some variable on your stack was overflowed or underflowed. Also that the program asserts when you leave the handler indicates that that the stack is corrupted in that function, not in the DoModal.

toxcct wrote:
::strcpy(m_strEditorVersion, EditorVersion);

I think that the problem is here. You call un unsafe C function on a variable with an uninitialized length! If my assumtions are correct, m_strEditorVersion is of type CString.

If I'm correct, see what the following gets you:

::strncpy(m_strEditorVersion.GetBufferSetLength(256), EditorVersion, 255);
m_strEditorVersion.ReleaseBuffer();

Your problem was because you were writing to a CString buffer that had a size which was too small. A better solution would be:

COptionsDlg::COptionsDlg(TEditorOptions& EditorOptions, <br />
                         CString         EditorVersion,<br />
                         CWnd*           pParent /* = NULL */)<br />
        : CDialog(COptionsDlg::IDD, pParent),<br />
          m_EditorOptions(EditorOptions),    // m_EditorOptions is defined as a TEditorOptions&<br />
          m_strEditorVersion(EditorVersion) <br />
{    // or just try:<br />
     // m_strEditorVersion = EditorVersion;<br />
     m_strDigitsInYears.Format("%d", m_EditorOptions._NbDigitsInYears);       <br />
     m_strDigitsInPhones.Format("%d", m_EditorOptions._NbDigitsInPhones);<br />
}<br />

May I ask why you use ::strcpy to copy CString contents?

Behind every great black man...
... is the police. - Conspiracy brother

Blog[^]

toxcct7-Jul-05 0:18

7-Jul-05 0:18

Bob Stanneveld wrote:
m_strEditorVersion(EditorVersion)
{ // or just try:
// m_strEditorVersion = EditorVersion;

in fact, i did this first, but the compiler told me to put this in the constructor initialization list...

Bob Stanneveld wrote:
May I ask why you use ::strcpy to copy CString contents?

i do ::strcpy(m_strEditorVersion, EditorVersion); because m_strEditorVersion is defined as a char[10]...
so, the code you suggest me with ReleaseBuffer() won't be able to work...

TOXCCT >>> GEII power
[toxcct][VisualCalc]

Bob Stanneveld7-Jul-05 0:26

Re: Run-Time Check Failure #2 [edited]

7-Jul-05 0:26

toxcct wrote:
i do ::strcpy(m_strEditorVersion, EditorVersion); because m_strEditorVersion is defined as a char[10]...
so, the code you suggest me with ReleaseBuffer() won't be able to work...

Your naming convention is a little bit misleading. m_strEditorVersion suggests that is is a normal string instead of a C-Style string. Use m_pszEditorVersion instead.

Back to the problem. Did you check the length of the EditorVersion string? You are just copying the string without length checking. This is a horrible mistake! You should use ::strncpy and copy up to 10 characters if you don't care about the '\0' or only 9 and set the NULL-terminator yourself. I'm almost 100% sure that this solves your problem.

Behind every great black man...
... is the police. - Conspiracy brother

Blog[^]

toxcct7-Jul-05 0:31

7-Jul-05 0:31

eBob Stanneveld wrote:
Did you check the length of the EditorVersion string? You are just copying the string without length checking

currently, yes you're right, but as i use the debugger to confirm, m_strEditorVersion is a char[10] and contains only "2.10" (correctly terminated with its trailing '\0' character)...

well, i'll test the changes on the afternoon, but now, i dont have enough time to tell you if the changes are useful or not. however, i think i'll change the char[10] to a CString, regardless to the memory occupied...

TOXCCT >>> GEII power
[toxcct][VisualCalc]

Bob Stanneveld7-Jul-05 0:32

7-Jul-05 0:32

Do you still get the error with the range checking code?

Behind every great black man...
... is the police. - Conspiracy brother

Blog[^]

toxcct7-Jul-05 5:32

7-Jul-05 5:32

hi bob, i'm back

i've edited my first post on this thread... the error remains the same, even when i changed the m_strEditorVersion into CString...

i think it is not the source of the problem because the Run-Time talks about TEditorOtions...

TOXCCT >>> GEII power
[toxcct][VisualCalc]

Bob Stanneveld7-Jul-05 5:45

Re: how to uninstall U disk

7-Jul-05 5:45

Hey,

toxcct wrote:
i've edited my first post on this thread... the error remains the same, even when i changed the m_strEditorVersion into CString...

At least that will not cause any problems in the future anymore. Big Grin | :-D

toxcct wrote:
i think it is not the source of the problem because the Run-Time talks about TEditorOtions...

I cannot see what the problem is because I don't know your code, but you could set a memory breakpoint at the address of the variable that gets overwritten. Your debugger will do its trick at the moment the memory is changed.
See here[^] and here[^] for more information about memory breakpoints.

Good luck! Big Grin | :-D

Behind every great black man...
... is the police. - Conspiracy brother

Blog[^]

how to uninstall U disk

6-Jul-05 18:35

Want to uninstall a U disk by VC++ program under windows2000

how to coding the program? have the api function about this?

David Crow7-Jul-05 5:07

David Crow

7-Jul-05 5:07

What is a "U disk?"

"Ideas are a dime a dozen. People who put them into action are priceless." - Unknown

switch() function

Anonymous6-Jul-05 18:35

Anonymous

6-Jul-05 18:35

I have created a simple sitch(), it takes integers

int choice;
switch(choice)
case 1:
break
.
.
default
message
break;

this works fine as long as the user enters a number (integers) but if I enter leters a,b,XD ..etc
it goes out in a loop.

how to chakec data type to ensure it is 1-9, if not display a message
Any way would be good
thanx

khan++6-Jul-05 19:22

khan++

6-Jul-05 19:22

Do this before your switch statement:
It just checks if the values are in the specified range.

if (choice >= 48 && choice <= 57)
{
switch (choice)
...
}

this is this.

Bob Stanneveld6-Jul-05 20:44

6-Jul-05 20:44

Anonymous wrote:
how to chakec data type to ensure it is 1-9, if not display a message

Your switch does this automatically with the default block. What else do you want?

Behind every great black man...
... is the police. - Conspiracy brother

Blog[^]

Anonymous7-Jul-05 8:18

Anonymous

7-Jul-05 8:18

Don't think you read the question right. The default works fine if an interger is entered (any number) but if a character is enter, it won't work!!!!!

Bob Stanneveld7-Jul-05 19:51

7-Jul-05 19:51

Anonymous wrote:
ForumVisual C++
Subject:Re: switch() function
Sender:Anonymous
Date:14:18 7 Jul '05

Don't think you read the question right. The default works fine if an interger is entered (any number) but if a character is enter, it won't work!!!!!

I don't think that you can program very well. That's OK since we are here to help.

I read the question and I can assure you that I understand it perfectly well! Your problem is that you don't know what you are doing. Let me elaborate: characters (I'll stick to single byte characters for simplicity) are represented by integral values in the range 0x0 to 0xFF. Integers (the usual 32bit integers) have values in the range 0x0 to 0xFFFFFFFF. What did we learn from this is that you can store an character into an int without problems. So a simple static_cast will solve your problem.

If you work with the datatype char however, the following code will do the trick:

std::cin >> cYourChar;
switch( cYourChar )
{
case '0': { /* code */ break; } // a 0 is entered
case '1': { /* code */ break; } // a 1 is entered
case '2': { /* code */ break; } // ...
case '3': { /* code */ break; }
case '4': { /* code */ break; }
case '5': { /* code */ break; }
case '6': { /* code */ break; }
case '7': { /* code */ break; }
case '8': { /* code */ break; }
case '9': { /* code */ break; }
default:
 // other character
}

Behind every great black man...
... is the police. - Conspiracy brother

Blog[^]