Thread: Seg Fault

Hey guys!

I have spent hours trying to figure out why this code works brilliantly until I add one function and then I get a seg fault. I know untracable/nonsensical error screams memory leak but I can not for the life of me find it. I free everything I need to and still have not idea.

I run this code and it works beautifully.

Code:

#include <stdio.h>#include <stdlib.h>
#include <string.h>
#include <assert.h>


#define LEN 20
#define MAXLL 20	//max length of each LL (ie. max number of identical hashes)
#define MAXARRY_SIZE 200


#define	DEBUG 1


struct node
{
	char *key;
	int info;
	struct node *next;
};


struct node *heads[LEN], *z;


void directions()
{
	printf("\n----------------Directions-----------------\n");
	printf("You may change the file hashme.txt to see what your\nkeys hash to. In	case there is a collision I keep the\nstring in a linked list and	just add a new node.\n");
}


void initHashList()
{
	int i;


	#ifdef DEBUG
		printf("\n**DEBUG: initHashList() running.....");
		printf("\nsizeof(*z) b/f malloc(): %li", sizeof(*z));
	#endif


	z = (struct node *) malloc(sizeof(*z));
	z->next = z; z->info = -1;
	
	for (i = 0; i < LEN; i++)
	{
		#ifdef DEBUG
			printf("\n%i) sizeof(*z) for heads[i]: %li", i, sizeof(*z));
		#endif
		heads[i] = (struct node *) malloc(sizeof(*z));
		heads[i]->next = z; heads[i]->info = -1;
		#ifdef DEBUG
			printf("\t**DEBUG: heads[i]: %u", &heads[i]);
		#endif
	}
}


//this hash fctn maps names to vals b/t 1 & 20
unsigned hash(char *v)
{
	int h;


	for (h = 0; *v != '\0'; v++)
	{
		h = (64*h  + *v) % 20;
	}
	return h;
}


void printLL(struct node *head)
{
	struct node *temp;
	temp = head;
	printf("%s", temp->key);
	while(temp->next->next != temp->next)
	{
		temp = temp->next;
		printf("->");
		printf("%s", temp->key);
		}
}


void printHashTable()
{
	int i;
	printf("\n---------------Hash Table--------------\n");
	for (i = 0; i < LEN; i++ )
	{
		printf("\nHash Val/Index: %i	", i);
		printLL(heads[i]);
	}
	printf("\n---------------------------------------\n");
}


int main()
{
	unsigned h;
	int i, j;
	struct node *temp, *x[MAXLL];
	char *str[] = {"someText", "sumText", "summoreTxt", "evnMoreTxt"};
	#ifdef DEBUG
		printf("\nsizeof(*temp):", sizeof(*temp));
	#endif
	temp = (struct node *) malloc(sizeof(*temp));


	directions();
	initHashList();


	i = 0;
	while(str[i++] != NULL)
	{
		h = hash(str[i-1]);
		if(heads[h]->info == -1) //no key entered with this value yet
		{
			heads[h]->key = str[i-1];
			heads[h]->info = 1;
		}
		else
		{
			//insert a new node just before z
			temp = heads[h];
			j = 0;
			while(temp->next->next != temp->next)	//ie. while node z != node z
			{
				temp = temp->next;
				j++;
			}
			x[j] = (struct node *) malloc(sizeof(*z));
			temp->next = x[j]; x[j]->next = z;
			x[j]->key = str[i-1]; 
			}
	}
	printHashTable();
	
	free(temp);
	for (i = 0; i < LEN; i++)
	{
		free(heads[i]);
		str[i];
	}
	return 0;
}

Then I add one function called readFile() and get a seg fault.

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>


#define LEN 20
#define MAXLL 20	//max length of each LL (ie. max number of identical hashes)
#define MAXARRY_SIZE 200


#define	DEBUG 1


struct node
{
	char *key;
	int info;
	struct node *next;
};


struct node *heads[LEN], *z;


void directions()
{
	printf("\n----------------Directions-----------------\n");
	printf("You may change the file hashme.txt to see what your\nkeys hash to. In	case there is a collision I keep the\nstring in a linked list and	just add a new node.\n");
}


void initHashList()
{
	int i;


	#ifdef DEBUG
		printf("\n**DEBUG: initHashList() running.....");
		printf("\nsizeof(*z) b/f malloc(): %li", sizeof(*z));
	#endif


	z = (struct node *) malloc(sizeof(*z));
	z->next = z; z->info = -1;
	
	for (i = 0; i < LEN; i++)
	{
		#ifdef DEBUG
			printf("\n%i) sizeof(*z) for heads[i]: %li", i, sizeof(*z));
		#endif
		heads[i] = (struct node *) malloc(sizeof(*z));
		heads[i]->next = z; heads[i]->info = -1;
		#ifdef DEBUG
			printf("\t**DEBUG: heads[i]: %u", &heads[i]);
		#endif
	}
}


//this hash fctn maps names to vals b/t 1 & 20
unsigned hash(char *v)
{
	int h;


	for (h = 0; *v != '\0'; v++)
	{
		h = (64*h  + *v) % 20;
	}
	return h;
}


void printLL(struct node *head)
{
	struct node *temp;
	temp = head;
	printf("%s", temp->key);
	while(temp->next->next != temp->next)
	{
		temp = temp->next;
		printf("->");
		printf("%s", temp->key);
		}
}


void printHashTable()
{
	int i;
	printf("\n---------------Hash Table--------------\n");
	for (i = 0; i < LEN; i++ )
	{
		printf("\nHash Val/Index: %i	", i);
		printLL(heads[i]);
	}
	printf("\n---------------------------------------\n");
}


void readFile( char fname[], char *str[])
{
	int i = 0;
	FILE * fp = fopen(fname, "r");
	char temp[200];


	#ifdef DEBUG
		printf("\n**DEBUG: readFile() running........");
	#endif

	assert(fp != NULL);	//file must != NULL to continue

	while(fgets(temp, 10, fp) != NULL)
	{
		#ifdef DEBUG
			printf("\n%i) sizeof(*char): %li", i, sizeof(char*)*strlen(temp+1));
		#endif
		str[i] = malloc(strlen(temp+1)*sizeof(char*));
		strncpy(str[i], temp, 10);
		i++;
	}
	#ifdef DEBUG
		printf("\n**DEBUG: done reading file....about to close file");
	#endif
	fclose(fp);
}


int main()
{
	unsigned h;
	int i, j;
	struct node *temp, *x[MAXLL];
        char *str[MAXARRY_SIZE];
	#ifdef DEBUG
		printf("\nsizeof(*temp):", sizeof(*temp));
	#endif
	temp = (struct node *) malloc(sizeof(*temp));


	directions();
	readFile("hashme.txt", str);
	initHashList();


	i = 0;
	while(str[i++] != NULL)
	{
		h = hash(str[i-1]);
		if(heads[h]->info == -1) //no key entered with this value yet
		{
			heads[h]->key = str[i-1];
			heads[h]->info = 1;
		}
		else
		{
			//insert a new node just before z
			temp = heads[h];
			j = 0;
			while(temp->next->next != temp->next)	//ie. while node z != node z
			{
				temp = temp->next;
				j++;
			}
			x[j] = (struct node *) malloc(sizeof(*z));
			temp->next = x[j]; x[j]->next = z;
			x[j]->key = str[i-1]; 
			}
	}
	printHashTable();
	
	free(temp);
	for (i = 0; i < LEN; i++)
	{
		free(heads[i]);
		str[i];
	}
	return 0;
}

In addition, make sure you're really freeing the memory.

Code:

	for (i = 0; i < LEN; i++)
	{
		free(heads[i]);
		str[i];  // <--- ???
	}

EDIT: Wow, beat to the punch 3 times!

It would help immensely if you provided a hashme.txt for us to test with.

Compile with your warnings set to the maximum, and fix them all, before you guess at what the problem is. It could be related to one of these:

Code:

$ make foo
gcc -Wall -Werror -ggdb3 -std=c99 -pedantic foo.c -o foo -lm -lpthread -lefence
foo.c: In function ‘initHashList’:
foo.c:40:9: error: format ‘%li’ expects argument of type ‘long int’, but argument 2 has type ‘unsigned int’ [-Werror=format]
foo.c:50:13: error: format ‘%li’ expects argument of type ‘long int’, but argument 3 has type ‘unsigned int’ [-Werror=format]
foo.c:55:13: error: format ‘%u’ expects argument of type ‘unsigned int’, but argument 2 has type ‘struct node **’ [-Werror=format]
foo.c: In function ‘readFile’:
foo.c:118:13: error: format ‘%li’ expects argument of type ‘long int’, but argument 3 has type ‘unsigned int’ [-Werror=format]
foo.c: In function ‘main’:
foo.c:138:9: error: too many arguments for format [-Werror=format-extra-args]
foo.c:178:9: error: statement with no effect [-Werror=unused-value]
cc1: all warnings being treated as errors
make: *** [foo] Error 1

Lines 40, 50 and 118: You should be using the right format specifier, probably %lu for unsigned long.
Line 55: I don't know what you're trying to do there, but if you want to print a pointer value, try %p.
Line 138: You need a %lu.
Line 178: What do you intend to do with that line? Do you mean to free(str[i])?

Note, your misuse of format specifiers for printf suggests that you should start reading documentation for the functions you use: printf(3): formatted output conversion - Linux man page.

As for your readFile function:

Code:

str[i] = malloc(strlen(temp+1)*sizeof(char*));

That's wrong in two places. First, the +1 belongs outside of the strlen call. Second, you have the wrong type in the parameter to sizeof. You want sizeof char. Or better yet, use the thing you are assigning the malloc to, with a * in front.

Code:

str[i] = malloc((strlen(temp) + 1) * sizeof(*str[i]));

The +1 inside the strlen actually takes the length of temp starting at the second char, so you end up two chars short of the desired length. That means every strncpy is a buffer overflow, you're accessing memory you don't own and probably trashing some of malloc's internal data, causing problems with malloc/free later on.

One more thing in your readFile function

Code:

char temp[200];
...
while(fgets(temp, 10, fp) != NULL)
...
    strncpy(str[i], temp, 10);

Why declare it 200 and then only use 10 of the chars. You should be using sizeof in the fgets call anyway, and there's no need for strncpy, you can just use strcpy (no 'n') since you (with the fix I suggested) will always be allocating the right amount of space for the string:

Code:

char temp[200];
while (fgets(temp, sizeof(temp), fp) != NULL)
...
    strcpy(str[i], temp);