'createfile ' method..file handling conflict / files overwriting each other

[peteB]

Hi,

I'm hoping this question is a recognisable issue to experienced windows programmers.It's the kind of thing that if you've experienced it you will recognise the behaviour described below. I think I am looking at a high level behaviour 'howler'
I have a program that uses the windows 'createfile' method for a number of
initialisation and logging features
It seems that I cannot have multiple files open at the same time. This is despite the files being managed by different instances of the file handling class.
The symptom is that the most recent file opened 'wins' and everything is streamed to / read from that file.
I don't have any statics in the header file of the class, the files are given unique names on opening.
There may be a clue in that am experiencing this wether the file is a com port or a disk file. I can have one com port and one disk file working simultaneously, no problem, but not more than one of each.
the com ports and disk files are managed by different classes (of course!)

hope someone can point me in the right direction

regards

Pete B

EDITED-- added code....


Header: nothing odd here?

#ifndef LOGGING_H_
#define LOGGING_H_

class Logging {
public:
Logging();
bool OpenLog (char * TimeStr);
bool OpenLogNotTime (char * FileName);
bool TimeStamp(void);
bool LogString(const char * LogEntry,DWORD entrylen);
bool LogNewline(void);
bool LogUint(unsigned int num);
bool LogULong32 (unsigned long num);
bool LogChar(const char * LogEntry);
bool LogByte(unsigned char num);
bool LogFloat(float num);
bool ClearLineBuffer(void);
bool AddToLineBuffer(const char * LineText);
bool LogLineBuffer(bool ClearWhenDone);
bool CloseLog(void);
virtual ~Logging();
};

#endif /* LOGGING_H_ */


and the implementation. -- not all of it,lots .... don't be mis directed by two means of opening file,its's just with or without time stamps, never will both be used in a given instance...



#include <windows.h>
#include <stdlib.h>
#include <stdio.h>
#include <time.h>
#include <string>
#include <sstream>

#include "Logging.h"

HANDLE LogFile =new HANDLE;
bool LogFileIsOpen;


char TheBuffer[500];
const char * LineBuffer; // probably don't really need to do it this way, but is safe...
int BuffPos;
/////////////////////////////////////////////////////////////
Logging::Logging() {
// TODO Auto-generated constructor stub
//LogFileIsOpen=0;
}
//////////////////////////////////////////////////////////////


///////////////////////////////////////////////////////////////////
bool Logging::OpenLog ( char *TimeStr ) //
{
DWORD dwBytes;
time_t rawtime;
struct tm * timeinfo;
char i;


char TimeBuff[30];

time ( &rawtime );
timeinfo = localtime ( &rawtime );
strcpy (TimeBuff,asctime (timeinfo)); // make a copy of asctime right away,



// convert the -'asctime' to a usable file name
for (i=5;i<23;i++)
{
TimeStr[i]=TimeBuff[(i-5)];
}
TimeStr[18]='_'; //(strip the ':' out)
TimeStr[21]='_'; //(strip the ':' out)
//TODO: above does make certain assumptions

if (LogFileIsOpen)
{
CloseHandle(LogFile);
LogFileIsOpen=0;
}

// printf ( "Current local time and date: %s", asctime (timeinfo) );

LogFile = CreateFile (
TimeStr,
GENERIC_WRITE,
0,
NULL,
CREATE_ALWAYS,
0,
NULL);

LogFileIsOpen=1;

i=TimeStamp();



}
//////////////////////////////////////////////
bool Logging::OpenLogNotTime (char * FileName)
{
if (LogFileIsOpen)
{
CloseHandle(LogFile);
LogFileIsOpen=0;
}

// printf ( "Current local time and date: %s", asctime (timeinfo) );

LogFile = CreateFile (
FileName,
GENERIC_WRITE,
0,
NULL,
CREATE_ALWAYS,
0,
NULL);

LogFileIsOpen=1;





}
/////////////////////////////////////////
bool Logging::TimeStamp(void)
{

DWORD dwBytes;
time_t rawtime;
struct tm * timeinfo;

if (LogFileIsOpen)
{
time ( &rawtime );
timeinfo = localtime ( &rawtime );
WriteFile(LogFile,asctime (timeinfo),19,&dwBytes,NULL );
WriteFile(LogFile,"\r\n",2,&dwBytes,NULL );
return 1;
}
return 0;
}

bool Logging::LogString(const char * LogEntry,DWORD entrylen)
{
DWORD dwBytes;
WriteFile(LogFile,LogEntry,entrylen,&dwBytes,NULL );
//WriteFile(LogFile,"\r\n",2,&dwBytes,NULL );

}


bool Logging::LogUint(unsigned int num)
{
DWORD dwBytes;
int length;
char IntAsString[16];

length=sprintf(IntAsString,"%d",(int)num);
// itoa( (int)num,IntAsString,10 );
dwBytes=(DWORD)length;
WriteFile(LogFile,IntAsString,length,&dwBytes,NULL );
//LogString(IntAsString,dwBytes);
}

[tath]

show us codeeee

[peteB]

code added....

[freddie1]

Hi PeteB,

Don't know if this comment will help, and I have to admit I didn't examine your code at all, but I did read your description of the problem.

For anything like this I don't use classes at all to handle files, COM Ports, or anything like that. In some cases I simply use the C Std. Lib. file handling functions, such as _wfopern() or fopen(). They are quite easy to use. And if you declare a unique FILE* for each file, no confusion will rersult.

The same exact thing is true with the proprietary CreateFile() function of Windows (which is likely how Microsoft implemented fopen()). In that case you end up with distinct HANDLEs, which is how one seperates the particular device, file, whatever.

When done in this manner, the types of wierd science you are experiencing doesn't happen. Cases such as these are cases where the higher level abstraction of C++ and classes isn't helping, but rather hindering one's efforts. All this in my opinion, of course.

[peteB]

Thanks freddie1.

Would that be pretty much the same as using ifstream, ofstream, etc?

[freddie1]

Well, I finally did look at the code. I see you are using CreateFile. This appears to be a problem...

HANDLE LogFile =new HANDLE;

Doesn't matter how many instances of the class you create, they all will use the same HANDLE, which is a big no, no. Every time you open a file it will overwrite the last handle to the opened file. The last one will be the only good one, as you are seeing.

[peteB]

Thanks so much Freddie1..

That does make sense, I appreciate that. But can I be sure what you are saying...
the Handle that I create at that point is then written over (re-created) by any subsequent instantiation of the class. That does match the behaviour observed, but doesn't seem to be what I expect c++ to do... if I created a static variable,pointer, whatever, in that class, it would be unique to that instance....

So there is something special about the 'HANDLE' type that I need to understand... I guess it's to do with windows interaction.
I appreciate the help you have given me, would me very grateful if you could confirm my understanding/comprehension of your answer.

Thanks again so much.

[freddie1]

I'm up at a Bed and Breakfast up in Vermont now but they have WiFi. That's why I didn't answer you right away.

But in any case, when I looked at your class Logging, all I'm seeing if I haven't missed anything are member function declarations. If you want to 'wrap' CreateFile() into a class (I just use it as is), then you really need to declare an instance variable (private, protected, public - your choice) of type HANDLE. In that case every time CreateFile() is called, whether it be through a constructor, a member function, or whatever, a unique file pointer, handle, whatever you want to call it, will be associated in a one to one relationship with the opened file stream. I have one program where I've opened five HANDLEs, and have never had happen what you are describing, but I keep careful track of each (opening and closing operations are critical).

Any type of HANDLE in Windows is something of an 'opaque pointer'. In your class it isn't achieving anything by declaring it as a pointer and allocating it with new. At least in my opinion. I'd just declare it as...

class Logging
{
 ...
 HANDLE hFile=NULL;
 ...
};

Hope this helps.

[peteB]

Freddie1,

Great, thanks, much appreciated. And certainly no need to explain yourself "That's why I didn't answer you right away." --- I don't take any of the help for granted, it will be repaid, but probably by me helping someone else in the future....
I'll let you know how I get on...

Thanks again...

Pete B.

[freddie1]

One other thing that is critical that I'm not sure you are doing is error checking. You absolutely have to test every CreateFile() call (also ReadFile()/WriteFile() or ReadConsole() WriteConsole()) for errors. The most important one is INVALID_HANDLE_VALUE.

[freddie1]

Really though, I do output logging like this ...

//Main.cpp
#ifndef UNICODE
#define UNICODE
#endif
#define MyDebug
#include <windows.h>
#include <cstdio>
#ifdef MyDebug
FILE* fp=NULL;
#endif

LRESULT CALLBACK fnWndProc(HWND hwnd, unsigned int msg, WPARAM wParam, LPARAM lParam)
{
 switch(msg)
 {
   case WM_CREATE:
     {
        #ifdef MyDebug
        fprintf(fp,"  Entering fnWndProc() -- case WM_CRETE\n");
        fprintf(fp,"    hwnd = %p\n",hwnd);
        #endif
        #ifdef MyDebug
        fprintf(fp,"  Leaving fnWndProc()  -- case WM_CREATE\n");
        #endif
        return 0;
     }
   case WM_DESTROY:
     {
        #ifdef MyDebug
        fprintf(fp,"  Entering fnWndProc() -- case WM_DESTROY\n");
        #endif
        PostQuitMessage(0);
        #ifdef MyDebug
        fprintf(fp,"  Leaving fnWndProc()  -- case WM_DESTROY\n");
        #endif
        return 0;
     }
 }

 return (DefWindowProc(hwnd, msg, wParam, lParam));
}


int WINAPI WinMain(HINSTANCE hIns, HINSTANCE hPrevIns, LPSTR lpszArgument, int iShow)
{
 wchar_t szClassName[]=L"Form1";
 WNDCLASSEX wc;
 MSG messages;
 HWND hWnd;

 #ifdef MyDebug
 fp=fopen("Output_Log.txt","w");
 fprintf(fp,"Entering WinMain()\n");
 #endif
 memset(&wc,0,sizeof(WNDCLASSEX));
 wc.lpszClassName = szClassName;                wc.lpfnWndProc=fnWndProc;
 wc.cbSize        = sizeof (WNDCLASSEX);        wc.hInstance=hIns;
 wc.hbrBackground = (HBRUSH)COLOR_BTNSHADOW;
 RegisterClassEx(&wc);
 hWnd=CreateWindowEx(0,szClassName,szClassName,WS_OVERLAPPEDWINDOW,75,75,320,305,HWND_DESKTOP,0,hIns,0);
 #ifdef MyDebug
 fprintf(fp,"  hWnd = %p\n",hWnd);
 #endif
 ShowWindow(hWnd,iShow);
 while(GetMessage(&messages,NULL,0,0))
 {
    TranslateMessage(&messages);
    DispatchMessage(&messages);
 }
 #ifdef MyDebug
 fprintf(fp,"Leaving WinMain()\n");
 fclose(fp);
 #endif

 return messages.wParam;
}

/*
Output:

Entering WinMain()
  Entering fnWndProc() -- case WM_CRETE
    hwnd = 002E0466
  Leaving fnWndProc()  -- case WM_CREATE
  hWnd = 002E0466
  Entering fnWndProc() -- case WM_DESTROY
  Leaving fnWndProc()  -- case WM_DESTROY
Leaving WinMain()
*/

[freddie1]

And I violated my cardinal principal above about error checking even though I used the C Std. Lib. file functions instead of CreateFile(). But consider doing what I say - not what I do! :)