Segmentation Fault (Can Someone look at my code?)

Hi Guys! This is my very first post :).

Need some help. I am working on this program, but I keep getting this segmentation fault. I know it has something to do with my memory allocation, but I can not get it to stop crashing. Any help would be nice.

Thanks in Advanced!

C code by jtkosh - 374 lines - codepad

1. Welcome to the forum. Please post your code here. Posting code? Read first.
2. Do not cast the return of malloc - Casting Malloc - FAQ
3. You should always check the return of malloc for success.
4. When you are allocating your pointers, you aren't allocating enough memory. e.g.
   
   Code:

   ---

   struct UserWorkCompleted* headWork= (struct UserWorkCompleted*) malloc(sizeof(struct UserWorkCompleted*));
// should be
struct UserWorkCompleted *headwork = malloc(sizeof(struct UserWorkCompleted));
// or preferably
struct UserWorkCompleted *headwork = malloc(sizeof(*headwork));

   ---

Make those changes and see if that resolves your problems. Note: The changes need to be on all your malloc calls.

Thanks for the quick reply! I will make the changes and see if that works.

Still not able to get it :-/....I made the malloc changes, but it's still giving me a run time error. Any more suggestions? Thanks!

Quote:

---

Originally Posted by jtkosh

Still not able to get it :-/....I made the malloc changes, but it's still giving me a run time error. Any more suggestions? Thanks!

---

Post your updated code here with [ code ] tags (read #1 of my reponse). You will get more responses. In the meantime, take a look at Lesson 15: Linked Lists and compare that to what you setup.

I use gcc, so first, I compiled your code with debugging symbols (-g) and warnings turned all the way up (-Wall):

Code:

---

$ gcc -g -Wall -o planet planet.c

---

Wow, 374 lines of code and not a single warning! Awesome! That's probably the first time I've seen that happen here on a code sample longer than 20 or so lines. Congrats.

Now, I ran your code through a debugger (gdb):

Code:

---

$ gdb ./planet
GNU gdb (GDB) Fedora (7.1-34.fc13)
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-redhat-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/charlesg/sandbox/test/planet...done.
(gdb) run
Starting program: /home/charlesg/sandbox/test/planet


Program received signal SIGSEGV, Segmentation fault.
__isoc99_fscanf (stream=0x0, format=0x8048f2b "%d") at isoc99_fscanf.c:31
31        _IO_acquire_lock_clear_flags2 (stream);
(gdb) backtrace
#0  __isoc99_fscanf (stream=0x0, format=0x8048f2b "%d") at isoc99_fscanf.c:31
#1  0x08048603 in main () at planet.c:89

---

Your file handle is NULL. You need to check for that before you go on trying to read from it. If you can't open your input or output files, you should probably bail out with an error. If we're going to help you, we need a sample input file. Also notice how a debugger helped me pinpoint the exact location of the seg fault. You should learn to use one, so you can find these things quickly and easily yourself.

Code:

---

        if (strcmp(curr->lastName, last))

---

Your condition is backwards. Check the documentation for strcmp, you'll see it returns 0 when they match (which C treats as false). EDIT: Nevermind, you just have containsUser return true if the list doesn't contain the user, backwards, but it works. I'd fix that for readability sake.

Make sure your names are at most 20 chars including the null, or you will have a buffer overflow when you strcpy in addUser.

Code:

---

int addUser(struct UserWorkCompleted* head, char last[], char first[], int work, int type)
{
    struct UserWorkCompleted* curr= head;
...

    while (curr != NULL)
    {
        curr=curr->next;
    }
...
    curr=temp;

---

Note, curr is just a pointer. Your loop walks it off the end of your list, so curr is just pointing at NULL. Then, you make it point at temp, but that doesn't actually add temp to the end of the list. You need to stop curr at the last element (when curr->next is NULL), and then do curr->next = temp to link it up.

Ok guys....still working away....It's due by midnite, so it has to get turned in one way or another :-/. But there is still hope!

I attached the sample in and out file. I had to change the names of the file to upload them. They are supposed to be in and out not txt.
Attachment 10918Attachment 10917

Thanks so much for all you guys help, this is really a warm welcome.

Code:

---

int addUser(struct UserWorkCompleted* head, char last[], char first[], int work, int type)
{
    struct UserWorkCompleted* curr= head;
...
   
    while (curr != NULL)
    {
        curr=curr->next;
    }
...
   
    curr=temp;
    temp->next= NULL;
   
    return 0;
}

---

Hi anduril462! Thanks for your help. I was thinking that the temp->next = NULL would take care of the last element link up?

Quote:

---

Originally Posted by jtkosh

Hi anduril462! Thanks for your help. I was thinking that the temp->next = NULL would take care of the last element link up?

---

Nope. It will only make the ->next element of the newly allocated node point to NULL. Effectively, you made temp a linked list of just one element. The general idea is

Code:

---

int addUser(struct UserWorkCompleted* head, char last[], char first[], int work, int type)
{
    struct UserWorkCompleted* curr= head;
    struct UserWorkCompleted* temp= malloc(sizeof(*temp));


    // normally you have a special case for when head is NULL (empty list), but you have a dummy node
    // at the beginning of your list, so we don't need to worry
    // when this loop terminates, curr will be pointing to the last node in your list
    while (curr->next != NULL)
        curr = curr->next;

    // fill in the details for our new node
    strcpy(temp->firstname, first);
    ...

    // now we can link up the new node
    curr->next = temp;

    // and make sure we have an end to our list
    temp->next = NULL;

    return 0;
}

---

Code:

---

struct UserWorkCompleted* temp= malloc(sizeof(*temp));]

---

When I make this change, like Hunter sugessted before....I am getting "headWork' undeclared (first use in this function)"

Post the new version of your code, with the line number of the error message.

Please post your updated code here like I asked you to. Nowhere in the one line you posted to you even mention headwork.

Code:

---

struct UserWorkCompleted* temp= malloc(sizeof(*temp));
    headWork->next= NULL;

---

107 C:\Users\*\Desktop\GridComputing.c `headWork' undeclared (first use in this function)

Sorry.....prob about to call it quits soon :-/....getting frustrated :(

Where do you define headWork in that function? That error is pretty plain, either there exists a variable named headWork in the function or not.