fgets reads whole file

[wackozacko]

Ok, so I'm trying to write an emulator for a simple processor architecture and I've ran into a strange problem. I decided to read the input code once through to see how many lines there were, then set up an array of that length, and then go through the file a second time to read the code into the array. Each line is 8 hex characters long.

The first time through, everything goes as normal, but the second time through, fgets reads the entire file, which is wierd.

It's been a while since I've programmed in C and even then I didn't do a whole lot of it, so it's probably something very simple, but I'm stumped. And google didn't help either.

Anyway, here's the relevant code.

void truncate(char *string)
{
	int l = strlen(string);
	if(string[l-1] == '\n')
		string[l-1] = '\0';
}

int main(int argc, char *argv[])
{
	if(argc < 2)		//check right number of arguments
		printf("USAGE: %s <input filename> <optional arguements>\n", argv[0]);
	else
	{
		int size = 0;
		int i = 0;
		
		FILE *inf;
		inf = fopen(strcat(argv[1],".o"), "r");		//open file
		if(inf == NULL)
		{
			fprintf(stderr, "ERROR: Can't open '%s' for reading!\n", argv[1]);
		}
		else
		{
			char varline[8];
			
			while(fgets(varline, 128, inf) != NULL)	//find file size
				size++;
			rewind(inf);
		
			char program[size][8];			//create array to hold all code
				
			for(i=0;i<size;i++)			//fill array from file
			{
				fgets(program[i], 128, inf);
				truncate(program[i]);		//remove newline character from string
			}
			fclose(inf);				//close file
		}
	}
}

For example. If the file has four lines of code

00000111
00005AB4
00006500
00009D01

printing out program[0] gives me

0000011100005AB40000650000009D01

and printing program[1] gives

00005AB40000650000009D01

P.S. I'm using the GCC compiler on a UNIX machine.

[Aia]

>>while(fgets(varline, 128, inf) != NULL)
varline can hold only 8 characters including the '\0', but you are saying to fgets() go and fetch 128.
sizeof operator can help you to avoid these pitfalls.
e.i.

fgets( varline, sizeof varline, inf) != NULL )

By the way, in C89 the char program[size][8]; needs to be declared in the beginning of the block. And it can not have a variable as the size of an array.

[wackozacko]

From what I understand of how fgets works, the number only specifies the maximum number of characters to copy in. It still shouldn't copy in more than one line. If I change the number to 8 it drops the last character off the string. If I change it to 9 or 10, it copies in the full line, but then copies a blank line into program[i+1]. As soon as I change it to 11 or above, it starts copying the whole file.

What I don't get is how that is possible, considering that I've specified both dimensions of the array.

Also, I cant declare program any sooner because I don't know the size until I've gone through the file at least once.

EDIT: I noticed one error. I have to declare the strings held in array program to be 9 characters long, 8 characters for the code and one character for the end of string '\0'. However, that still hasn't solved my problem for some reason. Now, instead of copying into program the entire file, it only copies in the last line. So every string in array program contains the line 00009D01.

[Aia]

>>Also, I cant declare program any sooner because I don't know the size until I've gone through the file at least once.
Then you must allocate dynamic memory for it.
The compiler must know the size of the array before run time.

>>From what I understand of how fgets works, the number only specifies the maximum number of characters to copy in.
It will read until reaches a newline or the EOF, and it will include the newline in the string. Nevertheless, asking for more than the buffer can hold is asking for trouble.
Each line in your file contains 9 characters, so to accommodate it, it requires a buffer of 9 + 1 for the '\0'.
You are stepping beyond the size of the buffer.
>> inf = fopen(strcat(argv[1],".o"), "r"); How do you know that argv[1] has room to append ".o"? No. no.

Edit: I removed some part of my comment.

[wackozacko]

Now that I know what I've done wrong, how do I actually fix it! I've not learned how to allocate memory (though I've come across the general syntax a couple of times) and I get the feeling it's a complex subject. The other two points you've made have no effect upon my original problem. fgets is still copying in only the last line of the file.

EDIT: sticking a print statement in the loop which fills in the array from the file, shows that within the loop things seem to be working fine. Outside the loop, things don't seem to work. It seems that fgets is working correctly and pulling in one line at a time and storing it in the current row in the array program. However, subsequent rows seem to be written over for one reason or another. I.E. it's a memory allocation problem. Since this is something I know nothing about, a bit of code that I can go over and try to understand would be usefull at this point.

[Aia]

>>The other two points you've made have no effect upon my original problem. fgets is still copying in only the last line of the file.
I am afraid you are failing to see the error of your logic. Your original problem would have been solved if you have made the change I suggested in my first post and reiterated in my last.

By allowing a maximum of 128 characters in fgets(), if your buffer is smaller than the capacity of the line + 1 (which it is) the '\0' terminator gets overwritten by suspensive reads and when you use printf("%s") there's no reference to that terminator in memory until it finds a '0' from the last string.

line 1 --> 00000111'\n' --> fgets read as much as 128 or until it finds a stopper ('\n' or EOF)
program[0][8];
program[0][0] = '0';
program[0][1] = '0';
program[0][2] = '0';
program[0][3] = '0';
program[0][4] = '0';
program[0][5] = '1';
program[0][6] = '1';
program[0][7] = '1'; --> End of capacity, but fgets() doesn't stop here. Still can continue writing. (max is 128)
program[1][0] = '\n'; --> placing it somewhere in adjacent memory in program[1]
program[1][1] = '\0'; --> Aha! fgets() stopped here

program[1][0] = '0'; --> Next line overwrite the '\n' or '\0' depends if it truncate already.
printf("%s", program[0]); --> doesn't have a NUL
program[0][6] = '1';

Use sizeof of as I stated before.

fgets( program[i], sizeof program[i], inf);

[wackozacko]

Actually, I have made every change as you have suggested it (save the memory allocation as I don't know how to do it). My problem remains.

CURRENT CODE:

//truncate removes a newline character from the end of a string
void truncate(char *string)
{
	int l = strlen(string);
	if(string[l-1] == '\n')
		string[l-1] = '\0';
}

int main(int argc, char *argv[])
{
	if(argc < 2)		//check right number of arguments
		printf("USAGE: %s <input filename> <optional arguements>\n", argv[0]);
	else
	{
		int size = 0;
		int i = 0;
		
		FILE *inf;
		inf = fopen(argv[1], "r");			//open file
		if(inf == NULL)
		{
			fprintf(stderr, "ERROR: Can't open '%s' for reading!\n", argv[1]);
		}
		else
		{
			char varline[9];
			
			while(fgets(varline, sizeof varline, inf) != NULL)//find file size
				size++;
			rewind(inf);				//return to start of file
		
			char program[size][9];			//create array to hold all code
			
			for(i=0;i<size;i++)
				program[i][0] = '\0';
				
			for(i=0;i<size;i++)			//fill array from file
			{
				fgets(program[i], sizeof program[i], inf);
				truncate(program[i]);		//remove newline character from string
			}
			fclose(inf);				//close file
			
			printf("%s\n", program[0]);
			printf("%s\n", program[1]);
			printf("%s\n", program[2]);
			printf("%s\n", program[3]);
		}
	}
}

INPUT FILE:

00000111
00005AB4
00006500
00009D01

OUTPUT:

00000111
BLANK LINE
00005AB4
BLANK LINE

[Aia]

Actually, I have made every change as you have suggested it (save the memory allocation as I don't know how to do it). My problem remains.

You missed one.

Each line in your file contains 9 characters, so to accommodate it, it requires a buffer of 9 + 1 for the '\0'.

instead of char program[size][9]; must be program[size][10];

[wackozacko]

I only want 8 characters from my file (the code is in 8 hex characters) so I'd already added one for the end of line character '\0'. Do you mean I need to add another to take into account the '\n' character at the end of every line in the file as well?

Ok, I'm going to think as I type here. When I read a line from the file what am I getting? 00000111\n or 00000111 ?

If I am getting 00000111 then I would want my variables to be 9 characters long, accounting for the 8 from file and the end of line '\0' character. Which is what I've got and it doesn't work, obviously.

If I am getting 00000111\n then I need my variables to be 10 characters long, accounting for the 9 from the file, plus the end of line '\0' character. Then using truncate() I remove the newline and replace with end-of-line.

HAHA! Second option worked, though I also had to change sizeof program[i] to sizeof program[i] + 1 (not sure why:icon_confused:).

Final code that seems to work.

//truncate removes a newline character from the end of a string
void truncate(char *string)
{
	int l = strlen(string);
	if(string[l-1] == '\n')
		string[l-1] = '\0';
}

int main(int argc, char *argv[])
{
	if(argc < 2)		//check right number of arguments
		printf("USAGE: %s <input filename> <optional arguements>\n", argv[0]);
	else
	{
		int size = 0;
		int i = 0;
		
		FILE *inf;
		inf = fopen(argv[1], "r");		//open file
		if(inf == NULL)
		{
			fprintf(stderr, "ERROR: Can't open '%s' for reading!\n", argv[1]);
		}
		else
		{
			char varline[10];
			
			while(fgets(varline, sizeof varline + 1, inf) != NULL)//find file size
				size++;
			rewind(inf);			//return to start of file
		
			char program[size][10];		//create array to hold all code
			
			for(i=0;i<size;i++)
				program[i][0] = '\0';
				
			for(i=0;i<size;i++)		//fill array from file
			{
				fgets(program[i], sizeof program[i] + 1, inf);
				truncate(program[i]);	//remove newline character from string
			}
			fclose(inf);			//close file
			
			printf("%s\n", program[0]);
			printf("%s\n", program[1]);
			printf("%s\n", program[2]);
			printf("%s\n", program[3]);
		}
	}
}

Command line to run the program a.out text.o text.o

00000111
00005AB4
00006500
00009D01

output

00000111
00005AB4
00006500
00009D01

Thanks loads for your help. I was definitely wandering down the wrong path. I'm sure there are a few more mistakes worth pointing out. I do appreciate the help. If I sounded short it's probably because this woke me up at midnight and I've been awake since (8 hours!) working on it.

Man, that was a lot longer and more difficult than it should have been!

[Aia]

HAHA! Second option worked, though I also had to change sizeof program[i] to sizeof program[i] + 1 (not sure why:icon_confused:).

No. No sizeof program[i] + 1 Do not fall again in the same pitfall. You do not want to give the chance to fgets() of stepping beyond the capacity of the buffer program. Think a little more about it. :)

[wackozacko]

Augh! But sizeof on it's own doesn't work! sizeof whatever + 1 does! It shouldn't but I'm just telling you what has worked. In fact, anything larger than just sizeof works.

[Aia]

Part of your problem could be as well the trucate() function.
What happens if you are giving it a malformed string, one that the '\0' is outside of the capacity of the buffer?
strlen() returns the length of the string by iterating through, until it finds a terminator.
If that terminator '\0' is beyond the boundary or is not there, what is the length returned by strlen()? Think about it.

[Aia]

Augh! But sizeof on it's own doesn't work! sizeof whatever + 1 does! It shouldn't but I'm just telling you what has worked. In fact, anything larger than just sizeof works.

Well, increase then the capacity of the buffer. Nothing wrong with making it program[size][20]

[wackozacko]

Problem identified and solved (I think).

fgets() grabs the whole line, including the newline character and then appends the string with an end-of-line character. This means that if my array is only 9 characters long, the end-of-line get's pushed into the next 'row' in the array. And things just go downhill from there.

Solution? fscanf. This only grabs characters up to a space or newline character and then appends an end-of-line character on the end. So I can specify my arrays to be 9 characters long and don't have to bother with specifying how many are read from the file. Plus, since it's not reading the newline character, I can get rid of my truncate function.

Tell me if this is right

int main(int argc, char *argv[])
{
	if(argc < 2)													//check right number of arguments
		printf("USAGE: %s <input filename> <optional arguements>\n", argv[0]);
	else
	{
		int size = 0;
		int i = 0;
		
		FILE *inf;
		inf = fopen(argv[1], "r");			//open file
		if(inf == NULL)
		{
			fprintf(stderr, "ERROR: Can't open '%s' for reading!\n", argv[1]);
		}
		else
		{
			char varline[9];
			
			while(fgets(varline, sizeof varline, inf) != NULL)//find file size
				size++;
			rewind(inf);				//return to start of file
		
			char program[size][9];		        //create array to hold all code
				
			for(i=0;i<size;i++)			//fill array from file
				fscanf(inf, "%s", program[i]);
			fclose(inf);				//close file
		}
	}
}

[Aia]

Problem identified and solved (I think).

fgets() grabs the whole line, including the newline character and then appends the string with an end-of-line character. This means that if my array is only 9 characters long, the end-of-line get's pushed into the next 'row' in the array. And things just go downhill from there.

Maybe my tedious work was not for naught, after all.

line 1 --> 00000111'\n' --> fgets read as much as 128 or until it finds a stopper ('\n' or EOF)
program[0][8];
program[0][0] = '0';
program[0][1] = '0';
program[0][2] = '0';
program[0][3] = '0';
program[0][4] = '0';
program[0][5] = '1';
program[0][6] = '1';
program[0][7] = '1'; --> End of capacity, but fgets() doesn't stop here. Still can continue writing. (max is 128)
program[1][0] = '\n'; --> placing it somewhere in adjacent memory in program[1]
program[1][1] = '\0'; --> Aha! fgets() stopped here

program[1][0] = '0'; --> Next line overwrite the '\n' or '\0' depends if it truncate already.
printf("%s", program[0]); --> doesn't have a NUL
program[0][6] = '1';

[...]
Solution? fscanf. This only grabs characters up to a space or newline character and then appends an end-of-line character on the end. So I can specify my arrays to be 9 characters long and don't have to bother with specifying how many are read from the file. Plus, since it's not reading the newline character, I can get rid of my truncate function.

Now a line is not a line but rather a word, considering that any whitespace will be a delimiter. Another different beast.