CODEWITHSUNDEEP
cpointersdata-structureslinked-listsingly-linked-list
mishelashala
mishelashala

Reputation: 541

Segmentation Fault - Object Oriented C - Linked List

I was trying to make a linked list using «Object Oriented» C. But I do not know how to backtrace segmentation fault, so I cannot find the solution.

Libraries, you know...

#include <stdio.h>
#include <stdlib.h>

Within the structure I have two pointers to functions. These pointers mimic methods (something like that).

struct nodeList {
  int data;
  struct nodeList * nextPtr;
  void (*print)(struct nodeList *);
  void (*insert)(struct nodeList **, int);
  int (*delete)(struct nodeList **, int);
};

You know, I hate write struct every time.

typedef struct nodeList NodeList;
typedef NodeList * NodeListPtr;

Prototypes declarations. init is something like a constructor. I do not need a destructor, the function free() handles that.

NodeListPtr init(int);
void print(NodeListPtr);
void insert(NodeListPtr *, int);
int delete(NodeListPtr *, int);

int main(void)
{
  NodeListPtr myList = init(0);

  myList->insert(&myList, 5);

Here is where the segmentation fault happens :(

  myList->insert(&myList, 3);

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  myList->print(myList);

  free(myList);
  return 0;
}

Function declarations, you know. init is, like I said, something like a constructor. It returns an instance of NodeListPtr with the pointer funcions pointing to the rest of the functions.

NodeListPtr init(int data)
{
  NodeListPtr instance = (NodeListPtr) malloc(sizeof(NodeListPtr));
  instance->data = data;
  instance->nextPtr = NULL;
  instance->print = &print;
  instance->insert = &insert;
  instance->delete = &delete;

  return instance;
}

Iterates over the list and prints all data.

void print(NodeListPtr listPtr)
{
  NodeListPtr currentPtr = listPtr;

  while(currentPtr != NULL) {
    printf("%d -> ", currentPtr->data);
    currentPtr = currentPtr->nextPtr;
  }

  puts("(NULL)");
}

Inserts an element in an orderly manner.

void insert(NodeListPtr *selfPtr, int value)
{
  NodeListPtr newPtr;
  NodeListPtr previousPtr;
  NodeListPtr currentPtr;

  newPtr = init(value);

  if(newPtr != NULL) {
    previousPtr = NULL;
    currentPtr = *selfPtr;

    while(currentPtr != NULL && value > currentPtr->data) {
      previousPtr = currentPtr;
      currentPtr = currentPtr->nextPtr;
    }

    if(previousPtr == NULL) {
      newPtr->nextPtr = *selfPtr;
      *selfPtr = newPtr;
    } else {
      previousPtr->nextPtr = newPtr;
      newPtr->nextPtr = currentPtr;
    }
  } else {
    printf("Could not add %d. No memory available\n", value);
  }
}

Deletes an element of the list. If it does not exist, return null character.

int delete(NodeListPtr *selfPtr, int data)
{
  NodeListPtr tempPtr;
  NodeListPtr currentPtr;
  NodeListPtr previousPtr;

  if((*selfPtr)->data == data) {
    tempPtr = *selfPtr;
    *selfPtr = (*selfPtr)->nextPtr;
    free(tempPtr);

    return data;
  } else {
    previousPtr = *selfPtr;
    currentPtr = (*selfPtr)->nextPtr;

    while(currentPtr != NULL && currentPtr->data != data) {
      previousPtr = currentPtr;
      currentPtr = currentPtr->nextPtr;
    }

    if(currentPtr != NULL) {
      tempPtr = currentPtr;
      previousPtr->nextPtr = currentPtr->nextPtr;
      free(tempPtr);

      return data;
    }
  }

  return '\0';
}

And that's it. Like I said, I do not know how to do backtrace over segmentation fault with GDB, so I do not know how to fix it.

Upvotes: 1

Views: 114

Answers (2)

Armali
Armali

Reputation: 19395

Replace

NodeListPtr instance = (NodeListPtr) malloc(sizeof(NodeListPtr));

with

NodeListPtr instance = (NodeListPtr) malloc(sizeof(NodeList));

in your code

– BLUEPIXY

Upvotes: 2

xception
xception

Reputation: 4297

Since your segmentation fault happens inside the insert function we can assume that one is to blame.

Problem seems to be in this code (didn't actually build the code... but by C rules the compiler is allowed to evaluate an expression in any order)

while(currentPtr != NULL && currentPtr->data != data) {
  previousPtr = currentPtr;
  currentPtr = currentPtr->nextPtr;
}

you code should instead be written as

while(currentPtr != NULL) {
    if(currentPtr->data != data) {
        previousPtr = currentPtr;
        currentPtr = currentPtr->nextPtr;
    } else {
        break;
    }
}

assuming that was your intent, didn't check the logic behind it actually, but the thing is otherwise you are dereferencing a null pointer there because although you do the check in order the compiler is free to evaluate it in any order it chooses to... so breaking it into a while and an if makes it so that your code is safe from dereferencing a null pointer.

Upvotes: 0

Related Questions