C help

rogan

rogan

rogan

Associate
Joined
15 Jul 2011
Posts
1,528
Location
London
Ive been stuck on a section of my C work for 6 hours now

its probobly something really obvious

http://pastebin.com/trVaftnY

it reads into a linked list (from the values at the top)

sorts them

outputs

simple in reality but i keep getting the valgrind error at the bottom of the PB

cheers guys!
 
Two bugs.

You aren't checking if you are at the end of your linked list when you traverse it, this leads to a null pointer being passed into compare_people which causes an explosion (invalid read).

Fix with:

Code: 
    while(people && compare_people(people, ptr) <= 0) {
      printf("%s comes before %s\n", people->name,ptr->name);
      people = people -> next;
    }


The second bug is in your for loop, again not checking if people->next is null or not. Strangely it breaks after the first iteration, and harriet is the first person printed (incorrect).

To fix this you need to keep track of the head of the list (first person), currently you overwrite the people pointer after each insert, which means you lose track of the everyone but the last person and the second loop traverse only finds 1 person.

Hope that helps with your homework! :) Also, I would use GDB instead of Valgrind for this kind of debugging.
 
Last edited:
Thanks very much, fixed a bunch of that, and created and end of list variable but im a bit confused how i would keep track of it.. as i dont just add to the end of the list at any point

edit i see the first not the last ok..

ill give that a go!
 
Last edited:
rogan said:
Thanks very much, fixed a bunch of that, and created and end of list variable but im a bit confused how i would keep track of it.. as i dont just add to the end of the list at any point

edit i see the first not the last ok..

ill give that a go!
Click to expand...


Could do something like this.. http://pastebin.com/vJTbgUq7 - the first time you call insert you stick the pointer in the firstperson variable.

It compiles and runs anyway :p, and fixes your other issue.

Differences: http://diffchecker.com/19TdCiE (scroll down)
 
Ok i added a first and now send the start of the list in my call as follows...

http://pastebin.com/7n8hMtR4

now i dont get a segfault but just

Person: Alfred, Age: 106
Person: Harriet, Age: 24
Person: Tim, Age: 32

in ouput. stumped.
 
Just a quick point, I couldnt get your code to run at first, since my compiler wasnt initialising the malloc'd area to 0, so you might want to add the line in your insert function,

Code: 
  if (temp == NULL) {
    first = ptr;
    lastperson = ptr;
[B]	ptr -> next = NULL;[/B]
    return ptr;
}

Just for general safety :).


Ok, so I got the same output as you, and the problem lies in your second 'else if' when we're running along your set of people, we're not keeping track of which person was the 'last person accessed'. So what happens is we get stuck adding to the second element alone, overwriting other stuff. I added a 'prevcheck' pointer to keep track and now it seems to be working :).

Code: 
static struct person *insert(struct person *temp, char *name, int age) {
  struct person *ptr, *prevcheck;
  ptr = (struct person *)malloc(sizeof(struct person));

  if(ptr== NULL)
    printf("No memory could be allocated!\n");

  ptr -> name = name;
  ptr -> age = age;

  if (temp == NULL) {
    first = ptr;
    lastperson = ptr;
	ptr -> next = NULL;
    return ptr;
    
  }else if (compare_people(temp, ptr) > 0) {
    if (lastperson == first) first = ptr;
    printf("adding %s before %s\n",ptr->name,temp->name);
    ptr -> next = temp;
    lastperson = ptr;
    return ptr;
  
  }else{
        
    while(temp && compare_people(temp, ptr) <= 0) {
      printf("%s comes before %s\n", temp->name,ptr->name);
      prevcheck = temp;
	  temp = temp -> next;
    }
      
     prevcheck -> next = ptr;
     ptr -> next = temp;
    return temp;
  
  }
  
}
 
You must log in or register to reply here.
Back
Top Bottom