#1
  1. The bad and the ugly...
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2007
    Location
    Oz... No??? Neverland then?
    Posts
    142
    Rep Power
    0

    Code crashes debugging help


    read in a .txt file with one letter per line. read each letter into an array. sort array alphabetically. print array to new .txt file


    Code:
    //precompile layer. cleaner code.
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    
    #define MAXCHAR 256 //size of memory location?? keeps code cleaner
    
    void sort(char *a, int size);
    //void swap(char *y, char *x);
    
    int main(void)
    {
    	//declaration variables
    	//
    	//for file opening
    	FILE *filename; 
    	FILE *filename_out;
    	char *fname = NULL;
    	//to append _out to end of filename
    	char *suffix = "_out.txt";
    	char *file_out = NULL;
    	char *ptr;
    	//
    	int n = 0; //used for prompting user for file name
    	int x = 0; //counter for length of file
    	int c = 0;
    	char temp; //
    	char *letters; //master array for characters to be sorted
    	//
    	//allocate memory for size of filename (char = 1 bit)
    	fname = (char *)malloc(MAXCHAR * sizeof(char));
    	
    	//
    	//prompt user for file name and continue to do so until a valid file name is given
    	while (n != 1)
    		{
    			printf("Enter file name: ");
    			scanf("%s", fname);
    			
    			//if statement w/ for loop inside of it
    			//go through each char, if \0 before . append ".txt" to the file
    			//else just open it
    			
    			filename = fopen(fname, "r"); //filename is just the handle to that file. have to go through filename for IO operation
    			
    			if (filename == NULL)
    				{
    					printf("Please enter a valid file name.\n");
    				}
    			else	
    				{
    					printf("File was successfully opened!\n");
    					n = 1;
    				}
    		}
    		
    		//
    		//while loop used to find the overall length of the file 
    		while (fscanf(filename, "%c\n", &temp) != EOF  ) //fscanf wants the address to store the character "&temp" not just "temp"
    			{
    				x++;
    			}
    		printf("%d\n", x);	
    		
    		//
    		//close and reopen file to move flag-bit to beginning of file
    		fclose(filename);	
    		filename = fopen(fname, "r");
    		
    		//
    		//allocate memory for master array based on file length (int x earlier)
    		letters = (char *)malloc(x * sizeof(char));
    		
    		//c = 0;
    		//read each character into array
    		while (fscanf(filename, "%c\n", &temp) != EOF)
    			{
    				*(letters + c) = temp;
    				c++;
    			}
    		
    		//
    		//create filename_out.txt
    		file_out = (char *)malloc((x) * sizeof(char));
    		strcpy( file_out, fname); //prefix
    		ptr = strchr(file_out, '.'); //search for the first occurance of '.' & deletes anything after it
    		strcpy( ptr, suffix); //append suffix to ptr
    		printf("%s\n", file_out);
    		
    		fclose(filename); //have to close previous file? program cannot open multiple files at the same time
    		
    		//test that file was created successfully
    		while (n != 0)
    			{
    				filename_out = fopen(file_out, "w");
    				if (fopen(file_out, "w") == NULL)
    					{
    						printf("The file could not be opened.\n");
    					}
    				else
    					n--;
    			}
    		
    		//
    		//bubble sort
    		sort(letters, x);
    		
    		//write sorted array to file
    				
    				
    		
    	//close file*_out and free allocated memory
    	fclose(filename);
    	free(fname);
    	free(file_out);
    	return(0);
    	
    }
    Code:
    #include <stdio.h>
    
    void swap(char *x, char *y);
    
    void sort(char *letters, int size)
    {
    	int i, j;
    		
    	for (i = 0; i < size; i++)
    		{
    			for (j = 0; j < size; j++)
    				{
    					if (*(letters + j) > *(letters + (j + 1)))
    						{
    							swap((letters + j), (letters + (j + 1)));
    						}
    				}
    		}
    	//*a = a[0]
    	//*(a+1) = a[1]
    	//*(a+2) = a[2]
    }
    Code:
    #include <stdio.h>
    
    void swap(char *x, char *y)
    {
    	char temp;
    	
    			temp = *y;
    			*y = *x;
    			*x = temp;
    
    }


    so I have a main method, which calls sort, which calls swap... lol kinda confusing but that's what the professor wants. Whenever I get to the bubble sort it crashes.

    Code:
    sort(letters, x);



    how can i debug? I've already done several test sorts with just random arrays that I create, and it sorts them perfectly! is there something wrong with the way I'm calling the funtions? btw, im compiling and running through command prompt and coding in notepad++.

    ~Tim
    "Life is not a journey with the intent on arriving at the finish line in a pretty and well preserved body. But rather to skid in broadside, totally worn out, thoroughly used up and loudly proclaiming, "Wow! What a ride!" -Anonymous
    Halo! || Diablo 2 LOD Modding || OLGA's BACK!
  2. #2
  3. Contributing User
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jan 2003
    Location
    USA
    Posts
    7,091
    Rep Power
    2222
    Using a debugger would help. But if you're using gcc, then the debugger is gdb, which I understand is not the easiest debugger to use. Without a debugger, you then need to resort to more primitive methods.

    What's in letters when you call sort? Verifying that you had filled in sort properly would be a first step. Also verifying that x contains the right count.

    If those look right, then you could insert printfs in the sort function to see what's happening in there. Those are called trace statements.

    Or you could sit down with pencil and paper and play computer by stepping through the sort function with the data you're feeding it. Or a smaller sample of data to make this task less tedious. This will force you to think through the mechanics of the code rather than look at it and think it looks right -- our minds tend to see what it expects to see and not what's actually there.


    Also, I would like to suggest some changes to your indenting style that should make your code more readable.

    First, I assume that in NotePad++ you have changed tabs to be something less than the default value of 8, maybe something like 3 or 4. But the problem with that is that that change only holds within the editor and when you then post that code elsewhere it goes back to 8 columns, as you can see when you look at your post. This is because your editor is inserting an actual tab character ('\x09') when you press the tab key. Most editors allow you the option of inserting spaces to create that indentation, so that when you copy your code elsewhere (such as to here), it retains the same formatting you had within your editor.

    Also, don't indent the braces as well as the code within them, but keep them at the same level as their control statements. For example, you wrote:
    Code:
    #include <stdio.h>
    
    void swap(char *x, char *y);
    
    void sort(char *letters, int size)
    {
    	int i, j;
    		
    	for (i = 0; i < size; i++)
    		{
    			for (j = 0; j < size; j++)
    				{
    					if (*(letters + j) > *(letters + (j + 1)))
    						{
    							swap((letters + j), (letters + (j + 1)));
    						}
    				}
    		}
    	//*a = a[0]
    	//*(a+1) = a[1]
    	//*(a+2) = a[2]
    }
    And we see the code marching off the right side of the display. Now look at this slight modification, just from not adding extra levels of indentation:
    Code:
    #include <stdio.h>
    
    void swap(char *x, char *y);
    
    void sort(char *letters, int size)
    {
    	int i, j;
    		
    	for (i = 0; i < size; i++)
    	{
    		for (j = 0; j < size; j++)
    		{
    			if (*(letters + j) > *(letters + (j + 1)))
    			{
    				swap((letters + j), (letters + (j + 1)));
    			}
    		}
    	}
    	//*a = a[0]
    	//*(a+1) = a[1]
    	//*(a+2) = a[2]
    }
    Wouldn't you agree that that looks a lot better and is more readable?

    Now for the final step, which is to use spaces instead of tabs so that we can keep the indentation down to 4 columns per level:
    Code:
    #include <stdio.h>
    
    void swap(char *x, char *y);
    
    void sort(char *letters, int size)
    {
        int i, j;
    		
        for (i = 0; i < size; i++)
        {
            for (j = 0; j < size; j++)
            {
                if (*(letters + j) > *(letters + (j + 1)))
                {
                    swap((letters + j), (letters + (j + 1)));
                }
            }
        }
        //*a = a[0]
        //*(a+1) = a[1]
        //*(a+2) = a[2]
    }
    Since formatting style can be something very personal, please keep in mind that these are just suggestions. But everything you can do to make your code more readable should be desirable.
  4. #3
  5. No Profile Picture
    I haz teh codez!
    Devshed Frequenter (2500 - 2999 posts)

    Join Date
    Dec 2003
    Posts
    2,540
    Rep Power
    2337
    gdb isn't really hard to use. You just need the manual and not to be scared by the command line.
    I ♥ ManiacDan & requinix

    This is a sig, and not necessarily a comment on the OP:
    Please don't be a help vampire!
  6. #4
  7. The bad and the ugly...
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2007
    Location
    Oz... No??? Neverland then?
    Posts
    142
    Rep Power
    0
    I couldn't even figure out how to install that haha. what extension is .gz or .sig file?

    Code:
    filename_out = fopen(file_out, "w");
    this line was making it crash. I read that fopen would open the file for "w" (write) and if it couldn't find it, it would create the file?
    "Life is not a journey with the intent on arriving at the finish line in a pretty and well preserved body. But rather to skid in broadside, totally worn out, thoroughly used up and loudly proclaiming, "Wow! What a ride!" -Anonymous
    Halo! || Diablo 2 LOD Modding || OLGA's BACK!
  8. #5
  9. Contributing User
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jan 2003
    Location
    USA
    Posts
    7,091
    Rep Power
    2222
    Isn't gdb part of the gcc tool-chain? At the command line, type gdb -v and you'll either get an error message that it couldn't find it or you'll get gdb's version information -- I don't have a Linux box handy at the moment and I'm not sure that my Windows port, "MinGW gcc", which does have gdb installed, would be typical.


    Whenever you use fopen, you should always test its return value for NULL. Even for output. Call it "defensive programming". And when it fails, you can use the perror() function to tell you why it failed -- you'll need to #include errno.h in order to use it.

    Hmm.
    Code:
            //test that file was created successfully
            while (n != 0)
            {
                filename_out = fopen(file_out, "w");
                if (fopen(file_out, "w") == NULL)
                {
                    printf("The file could not be opened.\n");
                }
                else
                    n--;
            }
    Since you're opening the file each time you loop and you never close it within the loop, I believe that it failed the second time through when you tried to open a file you already had open. Plus you were dropping your FILE pointer (dropped pointers are a bad thing).
  10. #6
  11. The bad and the ugly...
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2007
    Location
    Oz... No??? Neverland then?
    Posts
    142
    Rep Power
    0
    Originally Posted by dwise1_aol
    Since you're opening the file each time you loop and you never close it within the loop, I believe that it failed the second time through when you tried to open a file you already had open. Plus you were dropping your FILE pointer (dropped pointers are a bad thing).
    dropping my FILE pointer? I just took the
    Code:
    filename_out = fopen(file_out, "w");
    and it wouldn't even stand on it's own. maybe i missed an initialization or something?

    right now im more worried because i thought my program was reading the array correctly from the file... turns out its not :(

    a .txt like
    Code:
    a
    c
    d
    f
    e
    is reading in as
    Code:
    P
    Q
    R
    S
    T
    "Life is not a journey with the intent on arriving at the finish line in a pretty and well preserved body. But rather to skid in broadside, totally worn out, thoroughly used up and loudly proclaiming, "Wow! What a ride!" -Anonymous
    Halo! || Diablo 2 LOD Modding || OLGA's BACK!
  12. #7
  13. Contributing User
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jan 2003
    Location
    USA
    Posts
    7,091
    Rep Power
    2222
    Originally Posted by ChokolAwt
    dropping my FILE pointer? I just took the
    Code:
    filename_out = fopen(file_out, "w");
    and it wouldn't even stand on it's own. maybe i missed an initialization or something?
    Look at your while loop again:
    Code:
            //test that file was created successfully
            while (n != 0)  
            {
                filename_out = fopen(file_out, "w"); 
                if (fopen(file_out, "w") == NULL)   
                {
                    printf("The file could not be opened.\n");
                }
                else
                    n--;
            }
    Let's say that n = 2. You call fopen and assign the return value to filename_out. Then you call fopen again in the if-statement -- instead, you should have tested whether filename_out == NULL; anyway, I would expect that extra call to open the same file to fail. You print out the error message and go to the top of the loop, where you try to open the same file yet again, and again, and again, and again, ... .

    You should open the file once, before you enter the loop. If it fails, then you should exit the program after having printed an appropriate error message. After all, if you can't open the output file, then what more can you do?

    As for dropped pointers, here's an example:
    Code:
        char *sp;
    
        sp = (char*)malloc(42);
        sp = (char*)malloc(42);
    I just dropped that first pointer by assigning a different value to sp. Another example would be:
    Code:
        char *sp;
    
        sp = (char*)malloc(42);
        sp = NULL;
    Now, you already know to free malloc'd memory when you're finished with it. That's good. But do you understand why? Because when you run your program, the OS gives it a special segment of memory called the heap. malloc allocates from the heap and free returns memory back to the heap so it can be malloc'd again. But if you lose the address of memory that had been malloc'd, then you cannot free it because you have forgotten where it is. Think of holding plates and handing plates from one person to another. Each time you hand a plate to someone, he takes hold of it and then you let go of it. But if you let go of a plate before he take hold of it, then you have dropped the plate.

    The pointer returned by malloc is the only handle you have on that memory. If you assign a new address to that pointer without having saved the old address, then you have dropped the pointer. Dropped pointers cause memory leaks. You won't feel the effects of memory leaks in your short and short-running programs, but you will if your program is supposed to run constantly for days, months, or even years. If you have a memory leak, then that memory never gets freed up and cannot be reused. Heaps are big, but finite. After time, the amount of memory that cannot be freed will increase until the heap has no more memory to offer, so the next malloc call will fail and the program will crash. In the middle of the night, for no apparent reason. The problem is even worse in C++ where a lot of dynamic memory allocation and deallocation goes on behind the scenes. And that's all caused by dropped pointers.



    I haven't had time to look at why your code isn't reading the text file properly. Running it in a debugger would help immensely. Just today on a fairly large GUI project, we had an unexpected crash (so what crash is ever an expected one?). With the debugger, I found where it crashed and was able to back up from that point and view the values of local variables and I found the cause and fixed it within minutes. Debuggers are very valuable tools.
  14. #8
  15. The bad and the ugly...
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2007
    Location
    Oz... No??? Neverland then?
    Posts
    142
    Rep Power
    0
    Originally Posted by dwise1_aol
    As for dropped pointers, here's an example:
    Code:
        char *sp;
    
        sp = (char*)malloc(42);
        sp = (char*)malloc(42);
    I just dropped that first pointer by assigning a different value to sp. Another example would be:
    Code:
        char *sp;
    
        sp = (char*)malloc(42);
        sp = NULL;
    Now, you already know to free malloc'd memory when you're finished with it. That's good. But do you understand why? Because when you run your program, the OS gives it a special segment of memory called the heap. malloc allocates from the heap and free returns memory back to the heap so it can be malloc'd again. But if you lose the address of memory that had been malloc'd, then you cannot free it because you have forgotten where it is. Think of holding plates and handing plates from one person to another. Each time you hand a plate to someone, he takes hold of it and then you let go of it. But if you let go of a plate before he take hold of it, then you have dropped the plate.

    The pointer returned by malloc is the only handle you have on that memory. If you assign a new address to that pointer without having saved the old address, then you have dropped the pointer. Dropped pointers cause memory leaks. You won't feel the effects of memory leaks in your short and short-running programs, but you will if your program is supposed to run constantly for days, months, or even years. If you have a memory leak, then that memory never gets freed up and cannot be reused. Heaps are big, but finite. After time, the amount of memory that cannot be freed will increase until the heap has no more memory to offer, so the next malloc call will fail and the program will crash. In the middle of the night, for no apparent reason. The problem is even worse in C++ where a lot of dynamic memory allocation and deallocation goes on behind the scenes. And that's all caused by dropped pointers.
    that actually makes A LOT of sense. thank you. I'll work on it some more tonite (assuming i dont get too distracted by the DNC)
    "Life is not a journey with the intent on arriving at the finish line in a pretty and well preserved body. But rather to skid in broadside, totally worn out, thoroughly used up and loudly proclaiming, "Wow! What a ride!" -Anonymous
    Halo! || Diablo 2 LOD Modding || OLGA's BACK!
  16. #9
  17. Contributing User
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jan 2003
    Location
    USA
    Posts
    7,091
    Rep Power
    2222
    Of course it makes a lot of sense. And it becomes a very big issue when you go professional, since most of your software then will be expected to run full-time and error-free. And, as I said, it becomes especially important in C++.

    The main thing for you to take from this is to develop the discipline of free'ing everything that you malloc, and to never ever drop a pointer.

    Every programming language has a philosophy behind it. C was created for experienced programmers to work with. As a result, C will never try to protect you from yourself, unlike so many newer languages. C expects you to know what you are doing, so whatever you tell it to do, it will do, since you know what you are doing. This requires the C and C++ programmer to take on a lot of responsibility and requires that he have a more thorough knowledge of what his code does.

    A neophyte C programmer must learn from experience and from his seniors. There's not much more I can advise except for you to learn as we had all learned.

    Unfortunately, I had arrived at Trader Joe's in the middle of President Obama's speech. Damned good up to that point. After the party's convention, there's usually a "bump" in the polls in favor of that party's candidate. I heard that the Republicans got practically no bump. Let's see what the President gets in the next few days.

    A year or two after I turned 18, the voting age was lowered to 18. When it came time for me to register to vote, Nixon was in the White House. Mainly in reaction against Nixon, I registered as a Democrat. Then thereafter, everything I saw happening with the GOP told me that I had made the right decision. The irony is that both Nixon and Reagan would be considered too far to the left for their present party's current preference.

    Two more months with the future of our country hanging in the balance. One of the things that really nags at me is the sage advice that the people deserve whom they vote for. With so many people feeding off of Fox News etc, will they proceed to vote against their interests? Or vote for a future for our country? I'm old, due to retire/retire in 5 years, but it still scares me ****less that the Republicans might win.
  18. #10
  19. Contributing User
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jan 2003
    Location
    USA
    Posts
    7,091
    Rep Power
    2222
    Some programming advice. Here is how to test and handle successful opening of a file:

    Code:
    #include <errno.h>
    . . . 
    
        filename_out = fopen(file_out, "w"); 
        if (filename_out == NULL)   
        {
            fprintf(stderr, "fopen failed: %s\n", perror());
            exit(2);   // or whatever non-zero exit code you choose to indicate failure
        }
    Of course, if your program features recovery from such a failure, then you would not exit. The non-zero exit code indicates failure; UNIX/Linux shells and even Windows/DOS can use exit codes in shell scripts/batch files to test for success vs failure and decide what to do next accordingly.

    Also, there are two standard output streams, stdout and stderr. Normal output, such as from printf, goes to stdout, while error messages go to stderr. UNIX/Linux shells allow you to redirect stdout and stderr output separately; DOS is klunkier about that. You could printf error messages instead, but then you unnecessarily restrict your users' options.

    There's also a more idiomatic way to write that code that you will see often:
    Code:
    #include <errno.h>
    . . . 
    
        if ( ( filename_out = fopen(file_out, "w") ) == NULL)   
        {
            fprintf(stderr, "fopen failed: %s\n", perror());
            exit(2);   // or whatever non-zero exit code you choose to indicate failure
        }
    Read it and see that it does the same thing as the first example. It's very important that you get the parentheses right, so use that idiom only if you understand exactly what you're doing -- I find it useful to place the parentheses in first and then fill them with the assignment statement, just to ensure that I get them right. Also, the first way is easier to read, which would make it better for assignments you need to hand in. But the second example is so common among practicing programmers (and not necessarily among teachers -- sad fact, but too often true) that you will encounter it and so should know about it.

IMN logo majestic logo threadwatch logo seochat tools logo