Thread: Segmentation error

So I run this in the terminal, give it two filenames and I get a segmentation error, basically it's supposed to open two files, and print a line from file1 then print a line from file2 until an EOF. Where am I getting a pointer problem?

Code:

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#define MAX 1000

int main(int argc, char *argv[])
{
	int i = 0, j = 0;
	char ch, ct;
//	char fname[MAX];
	FILE *fo, *ft;
	char line1[MAX];
	char line2[MAX];
	
	if (argc != 3)
		{
		printf("Usage: %s filename \n", argv[0]);
		exit(1);
		}
	
	if ((fo = fopen(argv[1], "r")) == NULL)
		{
		printf("Can't open %s filename\n", argv[1]);
		exit(1);
		}
		
	if ((ft = fopen(argv[2], "r")) == NULL)
		{
		printf("Can't open %s filename\n", argv[1]);
		exit(1);
		}
	
	while ((ch = getc(fo)) != EOF && (ct = getc(ft)) != EOF)
		{
		i = j = 0;
		
		line1[i] = ch;
		i++;
		while ((ch = getc(fo)) != '\r')
			{
			line1[i] = ch;
			i++;
			}
		line1[i] = '\0';
		puts(line1);
		
		line2[j] = ct;
		j++;
		while ((ct = getc(ft)) != '\r')
			{
			line2[j] = ct;
			j++;
			}
		line2[j] = '\0';
		puts(line2);
		}
	
	fclose(fo);
	fclose(ft);
	
	return 0;
}

my C book says macintosh saves text files with \r at the end of lines (granted the book is old so maybe in OS X they may have changed it) and the 1000 MAX shouldn't be a problem since both files merely say

Code:

test test test
one two three

though I just changed it to \n and it prints the first line but then gives a segmentation error again before printing the "one two three" lines

There's at least one problem, in the loops which look like this:

Code:

while ((ch = getc(fo)) != '\r')
{
    line1[i] = ch;
    i++;
}

You're not checking for EOF. If you hit EOF before you see a '\r', this loop becomes infinite, and you overflow the array line1[]. Same for the other loop.

Even if you fixed that, you could still overflow, because you never check if i or j has reached MAX. You really want a loop like this:

Code:

/* Stop after MAX - 1 chars, so we have a char remaining for the null byte */
while((ch = getc(fo)) != EOF && ch != '\r' && i < MAX - 1)
{
    line1[i] = ch;
    i++;
}
line1[i] = '\0';
if(i == MAX - 1 && ch != EOF)
{
    /* The line is longer than the buffer. You need to discard characters until you reach
     * the end of the current line, otherwise you'll get out of sync
     */
}

that fixed it thanks, I figured that an EOF would always come right after a \n or \0 etc. bad assumption I guess. Thanks again guys

Moral: Never ASSUME that a file is correctly formed - always check for "stupid" things like this. It's bitten me before now.

--
Mats