Need a 2nd set of eyes on this. Crashing with fwrite().

[Matt Boudreaux]

This is fairly annoying and I'm probably missing something super simple. I've tried a variety of things to try and nail down the issue(simpler text file name, etc) but nothing works so far. If I use fopen instead of fopen_s, then the write succeeds but it crashes (invalid pointer error) on the fclose. Using fopen_s, I crash on the write command with an invalid pointer error. Return value from fopen_s is all fine, although the debugger is saying the write ptr is bad.

#include "LogManager.h"

#include <fstream>
#include <map>
#include <string>
#include <time.h>
#include <assert.h>

#include <Windows.h> // Needed for creating directory...

#define MAX_MESSAGE_LENGTH 4096

namespace LogManagerNamespace
{
	std::map<std::string, FILE *> ms_openedStreams;

	void EnsureDirectory(const char * directory);
}
using namespace LogManagerNamespace;


void LogManager::Log(const char * stream, const char * message, ...)
{
	std::string streamName = stream;

	std::map<std::string, FILE*>::iterator iter = ms_openedStreams.find(streamName);
	if(iter == ms_openedStreams.end())
	{
		FILE * newFile;
		std::pair<std::map<std::string, FILE*>::iterator, bool> retVal = ms_openedStreams.insert(std::make_pair(streamName, newFile));
		if(retVal.second)
		{
			iter = retVal.first;

			char nameBuffer[512];
			memset(nameBuffer, 0, 512);
			time_t timeNow = time(NULL);
			tm formattedTime;
			localtime_s(&formattedTime, &timeNow);
			EnsureDirectory(".\\Logs\\");
			sprintf_s(nameBuffer, 512, ".\\Logs\\%s_%Iu_%Iu_%Iu_%Iu.txt", streamName.c_str(), formattedTime.tm_hour, formattedTime.tm_min, formattedTime.tm_sec, static_cast<unsigned long long>(timeNow));

			errno_t fileStatus = fopen_s(&(iter->second), nameBuffer, "w");
			assert(fileStatus == 0);
		}

	}

	if(iter != ms_openedStreams.end())
	{
		char buffer[MAX_MESSAGE_LENGTH];
		memset(buffer, 0, MAX_MESSAGE_LENGTH);
		va_list args;
		va_start(args, message);
		vsprintf_s(buffer, message, args);
		va_end(args);
		fwrite(buffer, sizeof(char), strlen(buffer), iter->second); // Boom.
	}
}

[kbw]

Does buffer have reasonable contnet?

Also, you've opened the file. I'd prefer to write text with fprintf or fputs because of the end of line issues.

[modoran]

Did you check your iter->second FILE* pointer ?
Check your buffer content, try to not use strlen(buffer) and use a fixed small value to see if still crashes.

[bluecoder]

Hi ,
can any one explain me the line

std::pair<std::map<std::string, FILE*>::iterator, bool> retVal = ms_openedStreams.insert(std::make_pair(streamName, newFile));

What does this line do ?

[Pravesh Koirala]

I modified your program a bit (just to make it run. No actual changes done). And it worked just fine!!

///#include "LogManager.h
#include "stdafx.h"
#include <fstream>
#include <map>
#include <string>
#include <time.h>
#include <assert.h>
#include <cstring>
#include <cstdarg>
//#include <Windows.h> // Needed for creating directory...

#define MAX_MESSAGE_LENGTH 4096

namespace LogManagerNamespace
{
	std::map<std::string, FILE *> ms_openedStreams;

	void EnsureDirectory(const char * directory);
}
using namespace LogManagerNamespace;


void Log(const char * stream, const char * message, ...)
{
	std::string streamName = stream;

	std::map<std::string, FILE*>::iterator iter = ms_openedStreams.find(streamName);
	if(iter == ms_openedStreams.end())
	{
		FILE * newFile;
		std::pair<std::map<std::string, FILE*>::iterator, bool> retVal = ms_openedStreams.insert(std::make_pair(streamName, newFile));
		if(retVal.second)
		{
			iter = retVal.first;

			char nameBuffer[512];
			memset(nameBuffer, 0, 512);
			time_t timeNow = time(NULL);
			tm formattedTime;
			localtime_s(&formattedTime, &timeNow);
			//EnsureDirectory(".\\Logs\\");
			sprintf_s(nameBuffer, 512, "%s_%Iu_%Iu_%Iu_%Iu.txt", streamName.c_str(), formattedTime.tm_hour, formattedTime.tm_min, formattedTime.tm_sec, static_cast<unsigned long long>(timeNow));

			errno_t fileStatus = fopen_s(&(iter->second), nameBuffer, "w");
			assert(fileStatus == 0);
		}

	}

	if(iter != ms_openedStreams.end())
	{
		char buffer[MAX_MESSAGE_LENGTH];
		memset(buffer, 0, MAX_MESSAGE_LENGTH);
		va_list args;
		va_start(args, message);
		vsprintf_s(buffer, message, args);
		va_end(args);
		fwrite(buffer, sizeof(char), strlen(buffer), iter->second); // Boom.
	}
}

int main()
{
    Log("Pravesh", "Program started at %d Millisecond\n", clock());
    Log("Pravesh", "Now testing the log. Writing %d, %s \n",52,"Text");
    Log("Pravesh","Exiting goodbye!! %s\n","San fransisco");
}

Program started at 5204 Millisecond
Now testing the log. Writing 52, Text 
Exiting goodbye!! San fransisco

[bluecoder]

Hi ,
errno_t fileStatus = fopen_s(&(iter->second), nameBuffer, "w");how did
iter->second get decripted

Can some one help me ..
Thanks in advance .

[kbw]

That's a good point. You could do yourself a favor and avoid those secure versions of standard C functions. Personally, I never bother with them.

You need to do:

FILE *f;
if (fopen_s(&f, nameBuffer, "w") == 0)
    iter->second = f;

[bluecoder]

Thanks kbw .. i got it now .. thanks

[Matt Boudreaux]

Yea, fprintf fixed it, the write must have been going over the buffer somehow. Thanks all.