linked list (another problem)

[edenn1]

i have already posted here about my linked list facebook program
and i have anoter problem with the function :
void cancelFriends(int id1, int id2, PersonList* allPersons)

here is the structs again :

typedef struct Person {
    int id;
    char* name;
    struct PersonList* friends;
} Person;

typedef struct PersonList {
    Person* person;
    struct PersonList* next;
} PersonList;


this is what i have allready wrote in the function :

void cancelFriends(int id1, int id2, PersonList* allPersons)
{
    Person* friend1,*friend2;
    PersonList* temp1,*temp2;

    friend1 = getPersonById(id1,allPersons);
    if(friend1==NULL)
        puts(USER_NOT_EXIST );

    friend2 = getPersonById(id2,allPersons);
    if(friend2==NULL)
        puts(USER_NOT_EXIST );



    temp1=friend1->friends;
    temp2=friend2->friends;




        if(friend1->friends->person->id == friend2->id)
        {
            friend1->friends=friend1->friends->next;
            free(temp1);

        }

        while(friend1->friends->next && friend1->friends->next->person->id != friend2->id)
            friend1->friends=friend1->friends->next;
                if(!friend1->friends)
            puts(NOT_FRIENDS);
        else
        {
                if(friend2->friends->person->id == friend1->id)
            {
                friend2->friends=friend2->friends->next;
                free(temp2);

            }

            while(friend2->friends->next && friend2->friends->next->person->id != friend1->id)
                friend2->friends=friend2->friends->next;
            if(friend2->friends) 
                friend2->friends->next=friend2->friends->next->next;

        }



}


Person* getPersonById(int id, PersonList* list){

    while(list && list->person->id!=id)
        list=list->next;

    if(list->person->id == id)
        return list->person;    
    else
        return NULL;

please help me to make my cancling function works !!!
thank you!!

[Tom Gunn]

What is your canceling function supposed to do?

[wildgoose]

It's hard to tell from your code since not much whitespace thus hard to read but it appears you're using a linked node before verifying it even exists? You check the base node for NULL but not the friend pointer, you just use it!

if(friend1->friends->person->id

This isn't the fix, but may I suggest a slight rewrite on one of your functions...

Person * getPersonById( int id, PersonList* list)
{
   while( list )
  {
      if (list->person->id==id)
      {
         return list->person; 
      }

      list=list->next;
   }

   return NULL;
}

[edenn1]

thank you!
my function cancel friendship should use getpersonbyID to find to peple and cancel their friendship.
it should go to friend1 linked list of friend2 and delete the node of friend2 .
the same to friend2.
the function should check if friend1 and friend2 exist and also check if they are friends

[edenn1]

here is my function:

void cancelFriends(int id1, int id2, PersonList* allPersons)
{

    Person* friend1;
    Person* friend2;
    /*PersonList* temp2;
    PersonList* temp1;*/

    friend1=getPersonById(id1,allPersons);
    friend2=getPersonById(id2,allPersons);

    if( friend1==NULL || friend2==NULL )
        puts(USER_NOT_EXIST);
    else
    {
        if(friend1->friends->person->id==id2)
            free(friend1->friends);
        while(friend1->friends->next && friend1->friends->next->person->id!=id2)
            friend1->friends=friend1->friends->next;
        if(friend1->friends->next!=NULL)
            friend1->friends->next=friend1->friends->next->next;


        if(friend2->friends->person->id==id1)
            free(friend2->friends);
        while(friend2->friends->next && friend2->friends->next->person->id!=id1)
            friend2->friends=friend2->friends->next;
        if(friend2->friends->next!=NULL)
            friend2->friends->next=friend2->friends->next->next;
        else
            puts(NOT_FRIENDS);



    }


}

there is still one problem .
when i go with friend1->friends to the next node .
at the next time a loose the pointer to the head of the link .

how can i make it better!!!
thank you!!!!!

[wildgoose]

ADD whitespace to make this readable!
Stop using friend1, friend2 to refer to person links.

It's getting visually lost into the data member friends within the structure Person. Friend1 and Friend2 refere to person and friends refer to a list?

Search and Replace nd change them to another keyword.

Change immediately so this code makes more sense!
Call it 'list'since it points to one. But definitely not anything with the word friend in it! Next in the list to refer to the next list is fine.
COMMENT YOUR CODE!!!!

You'r releasing a list associating a person but you aren't releasing the person? Your code makes it look like you're leaking memory by losing it.