Like Tree2Likes
  • 1 Post By stahta01
  • 1 Post By Matticus

Seg Fault

This is a discussion on Seg Fault within the C Programming forums, part of the General Programming Boards category; Hey guys! I have spent hours trying to figure out why this code works brilliantly until I add one function ...

  1. #1
    Registered User
    Join Date
    Nov 2012
    Posts
    1

    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;
    }

  2. #2
    Registered User
    Join Date
    May 2009
    Posts
    2,552
    Nearly brain dead today; but, this was the first problem I saw
    Code:
    str[i] = malloc(strlen(temp+1)*sizeof(char*));
    Likely you was this
    Code:
    str[i] = malloc((strlen(temp)+1) * sizeof(char));
    Edit: added extra ()s

    sizeof(char*) is not likely what you want
    strlen(temp+1) is 2 chars shorter than needed.

    Tim S.
    Salem likes this.
    "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the universe trying to produce bigger and better idiots. So far, the Universe is winning." Rick Cook

  3. #3
    Registered User
    Join Date
    Jun 2011
    Posts
    2,292
    In addition, make sure you're really freeing the memory.

    Code:
    	for (i = 0; i < LEN; i++)
    	{
    		free(heads[i]);
    		str[i];  // <--- ???
    	}
    Salem likes this.

  4. #4
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,499
    Even better, operator precedence (though it's a moot point since sizeof(char) is 1 anyway).
    Code:
    str[i] = malloc( (strlen(temp)+1) * sizeof(char));
    > strncpy(str[i], temp, 10);
    You should just have a regular strcpy.
    strncpy will write 10 chars, regardless of the actual string length (or the memory allocated).
    You know you allocated the correct amount, so there is no issue just using strcpy.
    -- Function: char * strncpy (char *restrict TO, const char *restrict
    FROM, size_t SIZE)
    This function is similar to `strcpy' but always copies exactly
    SIZE characters into TO.

    If the length of FROM is more than SIZE, then `strncpy' copies
    just the first SIZE characters. Note that in this case there is
    no null terminator written into TO.

    If the length of FROM is less than SIZE, then `strncpy' copies all
    of FROM, followed by enough null characters to add up to SIZE
    characters in all. This behavior is rarely useful, but it is
    specified by the ISO C standard.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  5. #5
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,467
    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);

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. seg fault Need help
    By kiros88 in forum C Programming
    Replies: 3
    Last Post: 03-02-2010, 09:56 PM
  2. Getting a seg fault
    By ammochck21 in forum C Programming
    Replies: 11
    Last Post: 01-23-2009, 04:27 AM
  3. Seg fault
    By Tommo in forum C Programming
    Replies: 40
    Last Post: 08-28-2007, 07:12 AM
  4. im sorry my fault
    By mercuryfusion in forum A Brief History of Cprogramming.com
    Replies: 14
    Last Post: 04-23-2005, 11:28 AM
  5. segmentation fault and memory fault
    By Unregistered in forum C Programming
    Replies: 12
    Last Post: 04-02-2002, 10:09 PM

Tags for this Thread


1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21