0

I have a program that creates a sorted linked list which after testing it works in creating the list. My problem comes when attempting to remove an element in the list. The types are hidden which in the code you'll see the parameter void* newObj and the struct has a function pointer it uses to make the actual comparison.

My code segfaults and I believe it's because the head of the list isn't updating so a pass-by-value issue but I'm not sure how to fix it.

Function:

int SLRemove(SortedListPtr list, void *newObj)
{
    SortedListPtr temp;
    SortedListPtr temp2;

    if(list->CompareFcn(newObj, list->value) == 0)
    {
        if(list->flag == 1)
        {
            return 0;
        }
        temp = list;
        list = list->next;
        printf("(if)Address of object: %p with value %d, freed.\n", temp, *(int*)temp->value);
        free(temp);
        printf("New head of list is, address: %p, value: %d.\n", list, *(int*)list->value);
        return 1;
    }
    for (temp = list; temp != NULL; temp = temp->next)
    {
        printf("Address of TEMP: %p\n", temp);
        temp2 = temp->next;
        if(temp2->CompareFcn(newObj, temp2->value) == 0)/*value == *newObj*/
        {
            temp->next = temp2->next;
            printf("(loop)Address of object: %p, freed.\n", temp2);
            free(temp2);
            return 1;
        }
    }

    return 0;
}

Caller:

for (icount = 1; icount <= 3; icount += 1)
    {
        *ptr[icount-1] = icount;
        printf("Removing...\n");
        printf("Current head of list to be removed, address: %p, value: %d.\n", integerList, *(int*)integerList->value);
        if (SLRemove(integerList, ptr[icount-1]) == 1)
        {
            printf("SLRemove number %d, success!\n\n", icount);
            free(ptr[icount-1]);
        }
        else
        {
            printf("SLRemove number %d failed!\n\n", icount);
        }
    }

Valgrind:

Removing...
Current head of list to be removed, address: 0x51f1040, value: 1.
Comparing newObj: 1 and item: 1. Returning 0.
(if)Address of object: 0x51f1040 with value 1, freed.
New head of list is, address: 0x51f1140, value: 2.
SLRemove number 1, success!

==23304== Invalid read of size 8
==23304==    at 0x400826: main (main.c:63)
==23304==  Address 0x51f1040 is 0 bytes inside a block of size 32 free'd
==23304==    at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==23304==    by 0x400B8D: SLRemove (sorted-list.c:176)
==23304==    by 0x400805: main (main.c:60)
==23304== 
Pete Jodo
  • 465
  • 1
  • 7
  • 19
  • The answer is "yes", and [see here](http://stackoverflow.com/a/12449034/596781) for a general recipe, and also [here](http://stackoverflow.com/a/8435899/596781) and [here](http://stackoverflow.com/a/12059728/596781). – Kerrek SB Oct 07 '12 at 23:36

1 Answers1

1

You are modifying list inside SLRemove(), but since it is passed by value, that change never gets back to the caller. Try

int SLRemove(SortedListPtr *list_ptr, void *newObj)
{
  SortedListPtr list = *list_ptr;
  .
  .
  .
    *list_ptr = list;
    return 1;
  .
  .
  .
}

When you call it, use:

    if (SLRemove(&integerList, ptr[icount-1]) == 1)
Vaughn Cato
  • 63,448
  • 5
  • 82
  • 132
  • That'll also require changing the function signature to `SortedListPtr * list`, I think. – Kerrek SB Oct 07 '12 at 23:38
  • @KerrekSB: Ah, C, not C++.. my mistake. – Vaughn Cato Oct 07 '12 at 23:41
  • it doesn't compile. i get "sorted-list.c: At top level: sorted-list.c:162:28: error: expected ‘;’, ‘,’ or ‘)’ before ‘&’ token" but I don't get what it's complaining about, I only added the ampere sign – Pete Jodo Oct 07 '12 at 23:44
  • @PeteJodo: Yes, sorry. I thought it was C++ initially. I've updated my answer. – Vaughn Cato Oct 07 '12 at 23:45
  • @PeteJodo: Ampere? Like with electricity? Are you programming an electric kite? :-) (The symbol for Ampere is `A`, by the way.) – Kerrek SB Oct 07 '12 at 23:45
  • @Kerrek SB *facepalm* i meant ampersand – Pete Jodo Oct 07 '12 at 23:48
  • Actually the SortedListPtr is a typedef'd struct (typedef struct SortedList* SortedListPtr;) so can I do something like &(*list) ...that doesn't look right though – Pete Jodo Oct 07 '12 at 23:55