0

I'm trying to implement a function which is part of a linked list struct, but for some reason I keep ending up with a segmentation fault in my tests. I'm not sure if the fault is caused by my definition of compare in the list struct or if its how I have implemented it in the linked_list_create function. I have been sitting with this for quite some time using GDB to debug the code. One thing I can see is that current_entry in the test contains no values but I don't see how that could cause this issue.

//structs

union elem
{
  int key;
  char *value;
  void *extra;
};

typedef union elem elem_t;


#define int_elem(x) (elem_t) { .key=(x) }
#define val_elem(x) (elem_t) { .value=(x) }
#define ptr_elem(x) (elem_t) { .extra=(x) }

typedef struct link link_t;

typedef struct list list_t;

/// Compares two elements and returns true if they are equal 
typedef bool(*eq_function)(elem_t a, elem_t b);


struct link 
{
  elem_t element; //data    
  link_t *next; //link
};

struct list
{
    link_t *first;
    link_t *last;
    eq_function compare;
};

//Function

list_t *linked_list_create(eq_function compare)
{
  list_t *result = calloc(1, sizeof(list_t)); // allocated memory for list
  if (result)
  {
    result->first = result->last = calloc(1, sizeof(struct link));
    result->compare = compare; 
  }
  return result;
}

bool linked_list_contains(list_t *list, elem_t element)
{
  for (link_t *cursor = list->first->next; cursor; cursor = cursor->next)
    {
      if (list->compare(cursor->element, element) == 0) 
      {
        return true;
      };
    }

  return false;
}

void linked_list_append(list_t *list, const elem_t element)
{
  list->last->next = link_create(element, NULL);
  list->last = list->last->next;
}
///inserts a link in the list dependent on link index
void linked_list_prepend(list_t *list, const elem_t element)
{
  linked_list_insert(list, 0, element);
}

elem_t linked_list_remove(list_t *list, const int index)
{
  link_t *tmp = list->first; //tmp acts as a cursor

  for (int i = 0; i < (index-1); i++) 
  {
    tmp = tmp->next; //at end of loop tmp is the link we wish to remove
  };

  link_t *to_remove = tmp->next;
  elem_t returnval = to_remove->element;
  link_destroy(to_remove); //destroys the link
  return returnval;
}


///inserts a link in a list given an index
void linked_list_insert(list_t *list, const int index, const elem_t value)
{
  link_t *previous = list->first; //first link in list

  for (int i = 0; i < (index-1); i++)
  {
    previous = previous->next; 
  };

  previous->next = link_create(value, previous->next); //inserts created link
}

//Test

void test_remove1()
{
   list_t *list = linked_list_create(NULL);
    elem_t valtest = int_elem(7);
   for (int i = 0; i < 8; i++)
    {
     linked_list_append(list, int_elem(0));
    };
   linked_list_insert(list, 5, valtest);
   linked_list_remove(list, 5);
   bool result = linked_list_contains(list, valtest);
   CU_ASSERT_FALSE(result);
   linked_list_destroy(list);
}

The test is in CUnit and does not finish due to a segmentation fault at linked_list_contains, line 95.

davdavdav2
  • 13
  • 3
  • 1
    These macros #define int_elem(x) (elem_t) { .key=(x) } #define val_elem(x) (elem_t) { .value=(x) } #define ptr_elem(x) (elem_t) { .extra=(x) } only make your code unreadable. – Vlad from Moscow Oct 07 '21 at 19:40
  • 1
    "*I have been sitting with this for quite some time using GDB*". Good. So tell us what you found. As a minimum the debugger will give you the exact line of code that triggers the seg fault. So put that into your post. – kaylum Oct 07 '21 at 19:40
  • @davdavdav2 This statement result->first = result->last = calloc(1, sizeof(struct link)); within the function linked_list_create also does not make a sense. – Vlad from Moscow Oct 07 '21 at 19:44
  • @VladfromMoscow How so? It makes testing easier atleast, they simply return the int, ptr or val of an elemnt. – davdavdav2 Oct 07 '21 at 19:45
  • `linked_list_create(false);` You are setting the `compare` function to `false`. But `false` is not a function. How do you expect that to work? Why are you surprised it will crash when it uses a boolean as a function? – kaylum Oct 07 '21 at 19:45
  • @davdavdav2 It will be more clear and correct just to set them to NULL. – Vlad from Moscow Oct 07 '21 at 19:46
  • @kaylum The segmentation fault happens at linked_list_contains – davdavdav2 Oct 07 '21 at 19:46
  • @davdavdav2 Yeah but which line exactly? Programming is a precise task and you should give precise info. – kaylum Oct 07 '21 at 19:47
  • @kaylum sorry I added that while testing, it's meant to be NULL but I'm not sure to be honest. – davdavdav2 Oct 07 '21 at 19:48
  • NULL? How does that help? That will cause this line to deref a NULL pointer: `(list->compare(cursor->element, element)`. Why don't you set it to a proper compare function? – kaylum Oct 07 '21 at 19:49
  • @kaylum It happens at line 95 (I have other tests that come before it) – davdavdav2 Oct 07 '21 at 19:50
  • @davdavdav2 With the dummy node pointed to by the pointer last this statement list->last->next = link_create(element, NULL); does not make a sense. – Vlad from Moscow Oct 07 '21 at 19:50
  • @kaylum I don't know how to do it and I think that is whats causing the issue. – davdavdav2 Oct 07 '21 at 19:50

1 Answers1

0

linked_list_create(false) sets the compare function to false. But false is not a function so it is not surprising that it would crash when it tries to call the compare function.

typedef bool(*eq_function)(elem_t a, elem_t b);

That is the function prototype for the compare function. It means it is a function that takes two elem_t parameters and returns a bool. So you need to define a function of that type and set the list accordingly.

For example:

bool my_eq_function (elem_t a, elem_t b)
{
    // Implement real compare logic here
    return false;
}

Then set it with:

linked_list_create(my_eq_function);
kaylum
  • 13,833
  • 2
  • 22
  • 31