C / C++ / MFC

7-Feb-08 22:28

I created a very simple class (CLogFile) to enable me to trace events within my applications (It writes diagnostic messages to a hard disk file during exceution of the app) Admittedly, I have generally used this in the past with Dialog-based apps, but now I have included it in a Doc/View MDI app and have a peculiar problem.
I should explain that my CLogFile class is instantiated in the InitInstance function of the app, and a global pointer created pointing to the CLogFile object. In functions that I want to trace, I simply dereference the global pointer and thereby call the Add(CString szMsg) public member function to add the string to the file.
This all works fine in all locations in which I have used it to date, but, currently having a problem in OnDraw(), I tried using it there and got a 0xC0000005 access exception but (and this is the interesting bit) on the SECOND time through !! I get the same problem in OnPrepareDC(). I have now generated an vanilla (i.e. as initially produced by AppWizard) doc/view app and added the LogFile class, and get the same problem. It appears that the exception is being generated because the member variables in the CLogFile object have been corrupted. In an attempt to find the source of the corruption, I opened a memory window and displayed the area where the class members are located. The data is correct after exiting AfxWndProc() and then it enters (windows) assembler code. Still tracing, I get to a call where it won't step through to the target. Locating the target address, I find that the location is "mid-instruction". I have no doubt that this is then leading to the access exception. Can anybody give me a clue on how this corruption might be occuring ? I just can't understand why it works perfectly well in (supposedly) ALL other areas of the code, and yet fails in this way in these two functions. Any help/explanation would be greatly appreciated !

Doug

CPallini7-Feb-08 23:06

7-Feb-08 23:06

I think you need to post some code.
Smile | :)

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

Still learning how to code7-Feb-08 23:13

7-Feb-08 23:13

Here's the CLogFile class (.h and .cpp)

#if !defined(AFX_LOGFILE_H__55017EEE_12D9_4F35_9CBC_4C111B28982A__INCLUDED_)
#define AFX_LOGFILE_H__55017EEE_12D9_4F35_9CBC_4C111B28982A__INCLUDED_

#if _MSC_VER > 1000
#pragma once
#endif // _MSC_VER > 1000
// LogFile.h : header file
//

/////////////////////////////////////////////////////////////////////////////
// LogFile

class CLogFile : public CStatic
{
// Construction
public:
CLogFile();

// Attributes
public:

// Operations
public:

// Overrides
// ClassWizard generated virtual function overrides
//{{AFX_VIRTUAL(LogFile)
//}}AFX_VIRTUAL

// Implementation
public:
void Add(CString szMessage);
CFile fLogFile;
CString m_szRootPath;
void Start(BOOL bAppend);
CString m_szLogFile;
CString m_szLogFilePath;
CLogFile(CString szLogFile, BOOL bAppend = FALSE);
virtual ~CLogFile();

// Generated message map functions
protected:
//{{AFX_MSG(CLogFile)
// NOTE - the ClassWizard will add and remove member functions here.
//}}AFX_MSG

DECLARE_MESSAGE_MAP()
};

/////////////////////////////////////////////////////////////////////////////

//{{AFX_INSERT_LOCATION}}
// Microsoft Visual C++ will insert additional declarations immediately before the previous line.

#endif // !defined(AFX_LOGFILE_H__55017EEE_12D9_4F35_9CBC_4C111B28982A__INCLUDED_)

// LogFile.cpp : implementation file
//

#include "stdafx.h"
#include "LogFile.h"

#ifdef _DEBUG
#define new DEBUG_NEW
#undef THIS_FILE
static char THIS_FILE[] = __FILE__;
#endif

/////////////////////////////////////////////////////////////////////////////
// CLogFile

CLogFile::CLogFile()
{
}

CLogFile::~CLogFile()
{
}

BEGIN_MESSAGE_MAP(CLogFile, CStatic)
//{{AFX_MSG_MAP(LogFile)
// NOTE - the ClassWizard will add and remove mapping macros here.
//}}AFX_MSG_MAP
END_MESSAGE_MAP()

/////////////////////////////////////////////////////////////////////////////
// CLogFile message handlers

CLogFile::CLogFile(CString szLogFile, BOOL bAppend)
{
m_szRootPath = "C:\\";
m_szLogFile = szLogFile;
m_szLogFilePath.Format("%s\\%s.log",m_szRootPath,szLogFile);
Start(bAppend);
}

void CLogFile::Start(BOOL bAppend)
{
CString szCopyFilePath, szFrom, szTo;
szCopyFilePath.Format("%s\\%s.bak",m_szRootPath,m_szLogFile);

// Push down LIFO stack of LogFiles
for( int iIndex = 5; iIndex > 0; iIndex--)
{
szFrom.Format("%s%sLog%i.bak",m_szRootPath,m_szLogFile,iIndex-1);
szTo.Format("%s%sLog%i.bak",m_szRootPath,m_szLogFile,iIndex);
CopyFile(szFrom,szTo,FALSE);
}
// Copy .log to .bak0
szTo.Format("%s%sLog%i.bak",m_szRootPath,m_szLogFile,iIndex);
CopyFile(m_szLogFilePath,szTo,FALSE);

BOOL bRC;
if(bAppend)
bRC = fLogFile.Open( m_szLogFilePath, CFile::modeWrite );
else
bRC =fLogFile.Open( m_szLogFilePath, CFile::modeCreate | CFile::modeWrite );

CString szMsg;
if(bRC == 0)
{
szMsg.Format("File not found %s",m_szLogFilePath);
AfxMessageBox(szMsg);
}
CTime TimeNow = CTime::GetCurrentTime();
CString szDate = TimeNow.Format( "%A, %B %d, %Y" );
//ASSERT( s == "Friday, March 19, 1999" );

CString sMessage, sText;

if(!bAppend)
{
sMessage = "Start of log file ";
sText.Format("%s - %s%c%c",sMessage,szDate,'\r','\n');

fLogFile.Write(sText.GetBuffer(512),sText.GetLength());

sMessage = "-----------------------------------------------------";
sText.Format("%s%c%c",sMessage,'\r','\n');
fLogFile.Write(sText.GetBuffer(512),sText.GetLength());
}
fLogFile.Close();

}

void CLogFile::Add(CString szMessage)
{
fLogFile.Open( m_szLogFilePath, CFile::modeWrite );
fLogFile.Seek( 0,CFile::end);
CString szText;
// Get current time
CTime TimeNow = CTime::GetCurrentTime();
CString szTime;
szTime.Format("%02i:%02i:%02i",TimeNow.GetHour(),TimeNow.GetMinute(),TimeNow.GetSecond());
szText.Format("%s %s%c%c",szTime,szMessage,'\r','\n');
fLogFile.Write(szText.GetBuffer(512),szText.GetLength());

fLogFile.Close();
}

Doug

CPallini7-Feb-08 23:16

7-Feb-08 23:16

Why did it inherit from CStatic?

Still learning how to code7-Feb-08 23:58

7-Feb-08 23:58

CPallini wrote:
Why did it inherit from CStatic?

No reason - my ignorance in selection of base class when I wrote it (didn't know a lot then - still don't !!) Shouldn't be a problem tho', should it ??

Doug

CPallini8-Feb-08 0:05

8-Feb-08 0:05

Probably you don't need to inherit it at all.
I don't know if it is the problem, but I suggest you to drop that inherithance.
You have also to remove all the MESSAGE_MAP stuff.
Smile | :)

Matthew Faithfull8-Feb-08 0:09

8-Feb-08 0:09

DougButtimer wrote:
Shouldn't be a problem tho', should it ??

It shouldn't but it probably is, CStatic is an MFC class and encapsulates a Window/Ctrl as well. Firstly you don't need all that overhead and secondly that places all sorts of restrictions on what you can and can't do with the class and introduces assumptions on the part of exisitng MFC code about how your class will behave, even that its first data member will be a hWnd. I still don't understand exactly why you're seeing what you're seeing but I would definitely start with a properly 'vanilla' log file calss that doesn't inherit from an already complex MFC class.

Nothing is exactly what it seems but everything with seems can be unpicked.

Still learning how to code8-Feb-08 0:59

8-Feb-08 0:59

Yes, it would be good to eliminate that possibility - I'll regenerate the class without inheritance and see what happens ! Thanks for the suggestion !

Doug

Still learning how to code8-Feb-08 1:31

8-Feb-08 1:31

As suggested, I made the following three changes to CLogFile:-

1------------------------------------------------------------
class CLogFile //: public CStatic 20080208
{
// Construction

2--------------------------------------------------------------
// DECLARE_MESSAGE_MAP() 20080208

3-------------------------------------------------------------
/* 20080208
BEGIN_MESSAGE_MAP(CLogFile, CStatic)
//{{AFX_MSG_MAP(LogFile)
// NOTE - the ClassWizard will add and remove mapping macros here.
//}}AFX_MSG_MAP
END_MESSAGE_MAP()
*/

I am sorry to say the problem remains as before !!! Sigh | :sigh:

Doug

CPallini8-Feb-08 2:17

8-Feb-08 2:17

Could you also post the code wherein exception happens?

Still learning how to code8-Feb-08 2:37

8-Feb-08 2:37

I can't really post the code where the exception happens ! Execution proceeds through CLogFile, exits OnDraw() and enters basic Windows code As I said in my original posting, I traced through the code and eventually into (windows) assembler and hit that call where the debugger would not step to. A copy of this line in the debugger is:-

7E41B517 call 7E4194A4

and the target of the call is the last byte of the first instruction below (assuming the disassembly is correct) :-

7E4194A1 or eax,dword ptr [esi+edi*2-75h]
7E4194A5 inc esp
7E4194A6 and al,4
7E4194A8 int 2Bh

Up to this pooint the memory window shows that the CLogFile member variable locations hold correct values. Execution past this point results in corrupted values. I'm baffled !!

Doug

CPallini8-Feb-08 2:50

8-Feb-08 2:50

Perhaps posting the code inside OnDraw...

Still learning how to code8-Feb-08 3:23

8-Feb-08 3:23

(I don't know where my last posting went to !!!!)

I can't post the code where the 0xc00000005 exception occurs because it is somewhere in windows assembler (I assume !) Execution proceeds through CLogFile::Add() returns to OnDraw(), exits from that, exits from CView::OnPaint(), executes the CPaintDC destructor, exits OnWndMsg(), exits CWnd::WindowProc(), exits AfxCallWndProc(), exits AfxWndProc(), exits AfxProcBase() and then enters assembler and executes the following 4 code segments:-
1------------------------------------------------
7E418734 mov ecx,dword ptr fs:[18h]
7E41873B and byte ptr [ecx+0FB4h],0
7E418742 cmp dword ptr [esp+4],0DCBAABCDh
7E41874A jne 7E440378
7E418750 add esp,8
7E418753 pop ebx
7E418754 pop edi
7E418755 pop esi
7E418756 pop ebp
7E418757 ret 14h
2------------------------------------------------
7E418816 mov dword ptr [ebp-1Ch],eax
7E418819 or dword ptr [ebp-4],0FFh
7E41881D call 7E418841
7E418822 mov eax,dword ptr [ebp-1Ch]
7E418825 call 7E418600
7E41882A ret 20h
3---------------------------------------------------
7E41B4C0 mov dword ptr [ebp-1Ch],eax
7E41B4C3 mov eax,dword ptr [ebp-1Ch]
7E41B4C6 call 7E418600
7E41B4CB ret 14h
4-------------------------------------------------
7E41B50C push 0
7E41B50E push 0Ch
7E41B510 pop edx
7E41B511 lea ecx,[ebp-0Ch]
7E41B514 mov dword ptr [ebp-0Ch],eax
7E41B517 call 7E4194A4
----------------------------------------------------

The debugger will NOT step into this last call
The target of this call is the last byte of the first instruction below:-

7E4194A1 or eax,dword ptr [esi+edi*2-75h]
7E4194A5 inc esp
7E4194A6 and al,4
7E4194A8 int 2Bh
etc .......

This is clearly wrong, and will lead to mis-execution and presumably the 0xc000005 eventually

Doug

David Crow8-Feb-08 2:49

David Crow

8-Feb-08 2:49

DougButtimer wrote:
...currently having a problem in OnDraw()...

Could it be that OnDraw() is called so often that CLogFile code can't keep up?

"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

Still learning how to code8-Feb-08 5:19

non-const static member variale initialization

8-Feb-08 5:19

DavidCrow wrote:
Could it be that OnDraw() is called so often that CLogFile code can't keep up?

Don't see how - I'm running to a break point on the LogFile->Add() function within OnDraw(). Invoke second refresh of screen by maximising it, and off we go through the path that I described earlier and end up at that Call to bad assembler code. Are there any "no, no's " for code within OnDraw() or in OnPrepareDC() functions, or any timing restrictions (we obviously have to access a file) ???

Doug

George_George7-Feb-08 21:16

George_George

7-Feb-08 21:16

Hello everyone,

Now compiler prevents (at least in Visual Studio 2008) us from initialize non-const static member variable in this way,

Class Foo {
    statuc int i = 200; // compile error, must use const static int i = 200;
    ...
};

Anyone know why compiler prevents initialize non-const variable in this way? Any side effects or just a fixed rule from C++ Spec?

thanks in advance,
George

Matthew Faithfull7-Feb-08 22:13

7-Feb-08 22:13

As far as I know it's per the spec. Allowing intialization at declaration at all is a fairly recent concession, you couldn't do it in VC6. It makes sense to only allow static const initialisation as that is really the same as declaration.
In the past header files tended to be stuffed with namespace polluting #defines like
#define DEFAULT_ARRAY_SIZE 42.
Now you can add
static const int s_iDefaultArraySize = 200;
to your class declaration instead, much nicer Big Grin | :-D

Nothing is exactly what it seems but everything with seems can be unpicked.

George_George8-Feb-08 1:49

George_George

8-Feb-08 1:49

Hi Matthew,

1.

Matthew Faithfull wrote:
you couldn't do it in VC6

I am using Visual Studio 2008.

2.

Matthew Faithfull wrote:
namespace polluting #defines

Why it is namespace polluting? It is defined in the global/default namespace?

regards,
George

Matthew Faithfull8-Feb-08 2:29

8-Feb-08 2:29

George_George wrote:
Why it is namespace polluting? It is defined in the global/default namespace?

Exactly because it's defined in the global/default namespace and it is NOT really global or shouldn't be. What if I include a third party header file which also has
#define DEFAULT_ARRAYSIZE 400 - potential disaster.
One of the principles of good OO programming is 'locality of reference' which basically means a name should only be defined for the scope where it is meant to be used. That was the whole point of introducing namespace into C++ (which also didn't work in VC6) so that myLib::CString could exist in the same project as CString from MFC and the correct reference can always be resolved rather than refering to whichever one was included first or most recently.

Nothing is exactly what it seems but everything with seems can be unpicked.

led mike8-Feb-08 4:17

led mike

8-Feb-08 4:17

Matthew Faithfull wrote:
which basically means a name should only be defined for the scope where it is meant to be used.

No that is not what the Locality of Reference or Principle of Locality[^] is about at all.

In computer science, locality of reference, also known as the principle of locality, is the phenomenon of the same value or related storage locations being frequently accessed. There are three basic types of locality of reference: temporal, spatial and sequential:

led mike

Matthew Faithfull8-Feb-08 6:05

8-Feb-08 6:05

Four types, the fourth being codespace, scope and extent, the principle is exactly the same it's just another virtual dimensionality. Smile | :)

Nothing is exactly what it seems but everything with seems can be unpicked.

led mike8-Feb-08 7:22

led mike

8-Feb-08 7:22

Matthew Faithfull wrote:
Four types, the fourth being codespace

Do you have a citation for that or are you asking use to recognize you as a leader in the industry capable of establishing Principles?

led mike

George_George8-Feb-08 21:28

George_George

8-Feb-08 21:28

Thanks Matthew,

So, your answer to my original question is, it is according to C++ Spec that non-const static variable is not allowed to be initialized at the time when it is declared, right?

I am interested in what the Spec says, could you help to quote the related C++ Spec please? Smile | :)

regards,
George