Thread: Can't get fgets to work properly

  1. #1
    Registered User
    Join Date
    Nov 2008
    Posts
    10

    Can't get fgets to work properly

    Hello.
    I'm making a small application to make permutations of strings in order to find any anagrams. These permutations are then checked against a dictionary file using fgets. It compiles nicely but when it is run it just crashes.
    Here is the entire source code but I suspect only the "checkDict" function is faulty because everything runs just fine when I comment it out.
    Code:
    #include <stdio.h>
    #define VERSION 0.7
    #define MAX_DICT_LEN 100
    #define MAX_RESULTS 10
    
    int maxResultsCounter = 0;
    char *temp;
    FILE *dict;
    
    // check current permutation against entire dict.txt
    void checkDict(char *str){
    	while(fgets(temp, MAX_DICT_LEN, dict) != NULL)
    		if(strcmp(str, temp) == 0){
    			printf("%s\n", str);
    			maxResultsCounter++;
    		}
    }
    
    // swap *first and *second
    void swap(char* first, char* second){
    	char temp = *second;
    	*second = *first;
    	*first = temp;
    }
    
    // compute permutations
    int perm(char* set, int begin, int end){
    	int i, c = 0, range = end - begin;
    	if(range == 1){
    		
    		// MAX_RESULTS invariant
    		if(maxResultsCounter > MAX_RESULTS)
    			return 0;
    		//checkDict(set);
    	}
    	else
    		for(i = 0; i < range; i++){
    			swap(&set[begin], &set[begin+i]);	// first swap
    			perm(set, begin+1, end);			// recursion
    			swap(&set[begin], &set[begin+i]);	// swap back
    		}
    	return 0;
    }
    
    int main(int argc, char *argv[]){
    	
    	// test arguments
    	if(argc == 1 || argc > 3){
    		printf("AnagramAnalyzer (v. %2.1f)\n\n", VERSION);
    		printf("syntax: \n\t* <anagram> <args>\n\n");
    		printf("args: \n\t[-s] adds a space to the anagram\n");
    		return 0;
    	}		
    	
    	char *in = argv[1];
    	
    	// convert to lower case
    	int i, len = strlen(in);
    	for(i = 0; i < len; i++)
    		if(in[i] > 64 && in[i] < 91)
    			in[i] += 32;
    		
    	// check for additional arguments
    	if(argc == 3 && strcmp(argv[2], "-s") == 0){
    		in[len] = ' ';
    		in[len+1] = '\0';
    		len++;
    	}
    	
    	// open file stream
    	if((dict = fopen("dict.txt", "r")) == NULL){
    		printf("Couldn't locate dict file!\n");
    		return 0;
    	}
    	
    	// permutate and check dictionary
    	printf("Checking...\n");
    	perm(in, 0, len);
    	
    	// stop and close
    	fclose(dict);
    	return 0;
    }

  2. #2
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    You don't allocate any space to *temp. Try:

    Code:
    char temp[MAX_DICT_LEN+1];
    instead.
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  3. #3
    Registered User
    Join Date
    Nov 2008
    Posts
    10
    Thanks a lot MK27. It runs without crashing now.

    I still can't get it to work though. If I pass a permutation of a word I know is in the dict.txt file, or even the word itself, it doesn't print it.

    I'm pretty new to C and I can't really understand why it doesn't work. I may be missing something major though

  4. #4
    Registered User
    Join Date
    Aug 2005
    Location
    Austria
    Posts
    1,990
    The string returned by fgets still has a '\n' appended.
    That's why strcmp fails.
    Kurt

  5. #5
    Registered User
    Join Date
    Nov 2008
    Posts
    10
    Oh cool thanks Kurt.

    I tried doing something along the lines of

    Code:
    // check current permutation against entire dict.txt
    void checkDict(char *str){
    	while(fgets(temp, MAX_DICT_LEN, dict) != NULL){
    		int i;
    		for(i = 0; i < strlen(temp); i++)
    			if(temp[i] == '\n')
    				temp[i] = "";
    		if(strcmp(str, temp) == 0){
    			printf("%s\n", str);
    			maxResultsCounter++;
    		}
    	}
    }
    to remove the undesired '\n' but that won't compile. I tried with empty "" as well. That compiles but still doesn't work. It throws the warning: assignment makes integer from pointer without a cast.

  6. #6
    Registered User
    Join Date
    Aug 2005
    Location
    Austria
    Posts
    1,990
    try
    Code:
    if(temp[i] == '\n')
        temp[i] = 0;
    Kurt

  7. #7
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    How to "remove the \n from a line returned by fgets()" is in the FAQ.
    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.

  8. #8
    Registered User
    Join Date
    Nov 2008
    Posts
    10
    Thanks a lot to all of you. It finally works!

    I don't know if the thread is now closed or if people would like to help do a bit of optimization. It seems awfully slow with words over 4 characters.

    - lonkz

  9. #9
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    You're not still calling strlen() in your for loop are you?
    Because that would be unnecessary, and slow.
    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.

  10. #10
    Registered User
    Join Date
    Nov 2008
    Posts
    10
    I didn't realise I was doing that. Could you point out where?

  11. #11
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by lonkz View Post
    I didn't realise I was doing that. Could you point out where?
    Code:
    void checkDict(char *str){
    	while(fgets(temp, MAX_DICT_LEN, dict) != NULL){
    		int i;
    		for(i = 0; i < strlen(temp); i++)
    			if(temp[i] == '\n')
    				temp[i] = "";
    		if(strcmp(str, temp) == 0){
    			printf("%s\n", str);
    			maxResultsCounter++;
    		}
    	}
    }

  12. #12
    Registered User
    Join Date
    Nov 2008
    Posts
    10
    Oh yeah, nicely spotted. I seem to have been looking at the old code originally posted frantically trying to figure out where I made the mistake.

    Well needless to say it has been corrected. Thanks for helping out

  13. #13
    Registered User
    Join Date
    Nov 2008
    Posts
    10
    Many thanks to everyone who helped me. The application now works great and fast for smaller words. However it doesn't seem to like more than 5 characters. I get some pretty weird results.

    Here is the new code:
    Code:
    #include <stdio.h>
    #define VERSION 1.0
    #define MAX_DICT_LEN 20
    #define MAX_RESULTS 10
    
    int maxResultsCounter = 0;
    char temp[MAX_DICT_LEN + 1];
    FILE *dict;
    
    // check current permutation against entire dict.txt
    void checkDict(char *str){
    	
    	// open file stream and check it
    	if((dict = fopen("dict.txt", "r")) == NULL)
    		printf("Couldn't locate dict file!\n");
    	
    	// continuously read line from dict
    	int i;
    	while(fgets(temp, MAX_DICT_LEN, dict) != NULL){
    		int len = strlen(temp);
    		for(i = 0; i < len; i++)
    			if(temp[i] == '\n')
    				temp[i] = '\0';
    			
    		// compare str to the line read
    		if(strcmp(str, temp) == 0){
    			printf("%s\n", str);
    			maxResultsCounter++;
    		}
    	}
    }
    
    // swap *first and *second
    void swap(char* first, char* second){
    	char temp = *second;
    	*second = *first;
    	*first = temp;
    }
    
    // compute permutations
    int perm(char* set, int begin, int end){
    	int i, c = 0, range = end - begin;
    	if(range == 1){
    		
    		// MAX_RESULTS invariant
    		if(maxResultsCounter > MAX_RESULTS)
    			return 0;
    		checkDict(set);
    	}
    	else
    		for(i = 0; i < range; i++){
    			swap(&set[begin], &set[begin+i]);	// first swap
    			perm(set, begin + 1, end);		// recursion
    			swap(&set[begin], &set[begin+i]);	// swap back
    		}
    	return 0;
    }
    
    int main(int argc, char *argv[]){
    	
    	// test arguments
    	if(argc == 1 || argc > 3){
    		printf("AnagramAnalyzer (v. %2.1f)\n\n", VERSION);
    		printf("syntax: \n\t* <anagram> <args>\n\n");
    		printf("args: \n\t[-s] adds a space to the anagram\n");
    		return 0;
    	}		
    	
    	// initialize sequence
    	char *in = argv[1];
    	
    	// convert to lower case
    	int i, len = strlen(in);
    	for(i = 0; i < len; i++)
    		if(in[i] > 64 && in[i] < 91)
    			in[i] += 32;
    		
    	// check for additional arguments
    	if(argc == 3 && strcmp(argv[2], "-s") == 0){
    		in[len] = ' ';
    		in[len+1] = '\0';
    		len++;
    	}
    	
    	// permutate and check dictionary
    	printf("Checking...\n");
    	perm(in, 0, len);
    	
    	// stop and close
    	fclose(dict);
    	return 0;
    }
    I tried executing the application with "waters" as argument to see what I'd come up with. Here is a print out from my windows cmd:
    Checking...
    waters
    waster
    rawest
    Couldn't locate dict file!

    Now this seems odd to me. Why would it open a new file stream three times and then not be able to find the file on the fourth attempt?
    Everyone have been most helpful so far so I hoped there would be more words of wisdom to collect.

  14. #14
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    How many times do you try to open the file? Why would you try to open the file three times?

  15. #15
    Registered User
    Join Date
    Nov 2008
    Posts
    10
    I tried initialising the stream in the main method just once before calling any functions and that results in only one anagram being printed. This caused me to draw the assumption that every time I run the dictionary file through (ie. call the checkDict function) and either find something or not I need to reinitialise the file stream in order for it to begin from the very top again.
    Am I wrong in believing this?

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 3
    Last Post: 11-17-2008, 12:36 PM
  2. How properly inherit from template?
    By 6tr6tr in forum C++ Programming
    Replies: 118
    Last Post: 04-25-2008, 04:30 AM
  3. My monitor doesn't work with Ubuntu Linux.
    By indigo0086 in forum Tech Board
    Replies: 1
    Last Post: 07-12-2007, 10:59 AM
  4. fgets erroneoulsy returns null string
    By leonv in forum C Programming
    Replies: 3
    Last Post: 12-04-2006, 06:20 AM
  5. how fgets work ??
    By Nada in forum C Programming
    Replies: 1
    Last Post: 12-22-2001, 06:07 PM