Extending fprintf() with variable arguments causes crash

aatwo (81)
Hey guys. I through I had managed to successfully create a function that was able to extend the functionality of fprintf() but I can't figure out why it keeps crashing when I start using arguments...

eg: The following code will not crash...

1
2
3
4
5
6
7
8
9
10
11
12
13
 
int toLog(FILE *f, const char *buffer, ...){
	if(f != NULL){
		va_list arguments;
		va_start(arguments, buffer);
		int r = vfprintf(f, buffer, arguments);
		va_end(arguments);
	}
}
...
int main(){
	FILE *myFile = fopen("programLog.log", "a");
	toLog(myFile, "\n\nLog Entry Number 1.");
}


... BUT the following code will crash...
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
int toLog(FILE *f, const char *buffer, ...){
	if(f != NULL){
		va_list arguments;
		va_start(arguments, buffer);
		int r = vfprintf(f, buffer, arguments);
		va_end(arguments);
	}
}
...
int main(){
	FILE *myFile = fopen("programLog.log", "a");
	char myString[500];
	memset(myString, 0, 500);
	sprintf(myString, "The End.");
	toLog(myFile, "\n\nLog Entry Number 2 - %s", myString);
}


... Does anyone have any idea why this may be crashing ? Thanks in advance :)
Last edited on
kooth (746)
Perhaps your fopen is failing? You need to check for errors and NULL returns from fuctions!
kbw (9488)
Works under MSVC and GCC, so it must be in code you haven't posted.
aatwo (81)
In the toLog() function it checks to see if the file is NULL and skips the print if it is just incase :)

It should be fine regardless though. I swapped all of the calls to "toLog()" to "fprintf()" and it worked with no crashes!
kbw (9488)
IS the crashing code exactly the same as what you've published?
aatwo (81)
Not exactly. There is some inheritance involved:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
 

class parentObject {
	public:
		device(){};
		~device(){};

		void toLog(FILE *f, const char *buffer, ...){
			if(f != NULL){
				va_list arguments;
				va_start(arguments, buffer);
				int r = vfprintf(f, buffer, arguments);
				va_end(arguments);
			}
		}

		char errorDescription[500];
		char logFilePath[500];

}

class childObject  :  public parentObject {

	public:
		childObject(){
			memset(logFilePath, 0x00, 500);
			sprintf(logFilePath, "programLog.log");
		};
		~childObject(){};

		void setErrorDescription(int code){
			memset(errorDescription, 0x00, 500);

			switch(code){
				case 0: sprintf(errorDescription, "logical error"); break;
				case 1: sprintf(errorDescription, "read error"); break;
				case 2: sprintf(errorDescription, "write error"); break;
				default: sprintf(errorDescription, "unknown error");
			}

			FILE *f = fopen(logFilePath, "a");
			toLog(f, "\n\nError description: %s.", errorDescription);
			if(f != NULL) fclose(f);
		}
}

int main(){

	...
	// do some code and obtain an error code
	int errorCode = 2; //  <- for example
	...

	parentObject *d = NULL;
	d = new childObject();
	if(d != NULL){
		d->setErrorDescription(errorCode);
		delete d;
	}
	return 1;
}


This is the exact code crashing.


Everything works perfectly with no problems what so ever until the toLog function is called with some arguments. If it is subsequently replaced by the corresponding fprintf() command it does not crash :(
Last edited on
aatwo (81)
I am compiling this on Debian and running it on a different linux machine... could this be a problem? There are no shared libraries being used and it is all being compiled into a single binary so it should be ok.
kbw (9488)
No, the platform shouldn't matter.

The code is very Java-esq.

Public data being modified by a derived class--you're asking for trouble.

It runs ok for me, I modified it to get it to run. Modifications is bold as far as I can remember.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
 
#include <stdio.h>
#include <memory.h>
#include <stdarg.h>

class parentObject
{
public:
	parentObject()
	{
	}
	~parentObject()
	{
	}

	void toLog(FILE *f, const char *buffer, ...)
	{
		if (f != NULL)
		{
			va_list arguments;
			va_start(arguments, buffer);
			int r = vfprintf(f, buffer, arguments);
			va_end(arguments);
		}
	}

	char errorDescription[500];
	char logFilePath[500];
};

class childObject : public parentObject
{
public:
	childObject()
	{
		memset(logFilePath, 0x00, 500);
		sprintf(logFilePath, "programLog.log");
	}
	~childObject()
	{
	}

	void setErrorDescription(int code)
	{
		memset(errorDescription, 0x00, 500);

		switch(code)
		{
		case 0: sprintf(errorDescription, "logical error"); break;
		case 1: sprintf(errorDescription, "read error"); break;
		case 2: sprintf(errorDescription, "write error"); break;
		default: sprintf(errorDescription, "unknown error");
		}

		FILE *f = fopen(logFilePath, "a");
		toLog(f, "\n\nError description: %s.", errorDescription);
		if (f != NULL)
			fclose(f);
	}
};

int main()
{
	// do some code and obtain an error code
	int errorCode = 2; //  <- for example

	childObject *d = NULL;
	d = new childObject();
	if(d != NULL)
	{
		d->setErrorDescription(errorCode);
		delete d;
	}

	return 1;
}


abellia (29)
The file probably isn't opening. Check that f isn't NULL BEFORE calling toLog().
kbw (9488)
The code deals with a NULL file handle.
aatwo (81)
Weird. After a lot of testing, it turns out that if I call my toLog() function in the same function as a different function called setStatus(), (that was being called higher up in my code)... the program crashed...


I.E:

THIS WILL CRASH
1
2
3
4
5
6
7
8
9
 
void myFunction(){
	char status[500];
	memset(status, 0, 500);
	sprintf(status, "Alive.");
	
	setStatus(status);
	
	toLog(m_plog, "\n\nSet status to '%s'.", status);
}


THIS WILL NOT CRASH
1
2
3
4
5
6
7
8
9
 
void myFunction(){
	char status[500];
	memset(status, 0, 500);
	sprintf(status, "Alive.");
	
	setStatus(status);
	
	//toLog(m_plog, "\n\nSet status to '%s'.", status);
}


THIS WILL NOT CRASH EITHER
1
2
3
4
5
6
7
8
9
 
void myFunction(){
	char status[500];
	memset(status, 0, 500);
	sprintf(status, "Alive.");
	
	//setStatus(status);
	
	toLog(m_plog, "\n\nSet status to '%s'.", status);
}


Obviously something is wrong with either the toLog() or the setStatus() function and I'm guessing its the setStatus function since I have basically seen the same code in my toLog() function all over the internet! But why is it that individually they cause no harm, but combined they crash?! Speculations? :D

For some reason if I wrap my setStatus() function in another function I am able to stop the program from crashing...

I.E.

1
2
3
4
5
6
7
8
9
10
11
12
13
 
void setStatusWrapper(char *buffer){
	setStatus(buffer);
}

void myFunction(){
	char status[500];
	memset(status, 0, 500);
	sprintf(status, "Alive.");
	
	setStatusWrapper(status);
	
	toLog(m_plog, "\n\nSet status to '%s'.", status);
}


...But I don't like doing it that way! No matter how much duct tape you use, eventually that car wheel will fall off and cause a crash!!! [/metaphor]
Last edited on
kbw (9488)
Can you post the code for setStatus, the problem's there. I think toLog is ok.
abellia (29)
BTW, you weren't handling a NULL file handle:

1
2
3
 
FILE *f = fopen(logFilePath, "a");
toLog(f, "\n\nError description: %s.", errorDescription);
if(f != NULL) fclose(f);


We can't help you fix your problem if you don't include all the relevant code.
aatwo (81)
Sure :) Note that the code in setStatus was not written by me! It is part of a program I am extending.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
 
std::map<std::string,std::string> m_statusvars;

// Global functions defined in main.cpp
void exportStatusVars(char *pwrdev){
	FILE* statusfile;
	char filename[15];
	std::map<std::string,std::string>::iterator it;

	memset(filename,'\0',15);

	// Write status to status file
	sprintf(filename,"status%c",pwrdev[strlen(pwrdev)-1]);
	statusfile = fopen(filename,"w");
	if (statusfile==0) return;

	for (it = m_statusvars.begin(); it != m_statusvars.end(); it++) {
		fprintf(statusfile,"<%s>%s</%s>\n",(*it).first.c_str(),(*it).second.c_str(),(*it).first.c_str());
	}

	fclose(statusfile);

	// Read pid from file and signal
	statusfile = fopen("net.pid","r");
	if (!statusfile) return;
	int pid;
	fscanf(statusfile,"%d",&pid);
	fclose(statusfile);
	kill(pid,SIGTSTP);
}

void setStatusVar(const  char* var, const char* value, bool commit, char* pwrdev){
	m_statusvars[var] = value;
	if(commit) exportstatusvars(pwrdev);
}

void setStatus(const char* status, char *pwrdev){
	setStatusVar("STATUS", status, true, pwrdev);
}


This is the exact code with no modifications :)
Last edited on
Topic archived. No new replies allowed.