Creating a singly linked list with 2 structures in C

Question

I am working with these two structures, the first holds the employee information while the second holds the list information:

typedef struct ListNodeTag{
  int idNumber;
  struct ListNodeTag *next;
} Employee;

typedef Employee Item;

typedef struct {
  int size;
  Item *head;
} List;

When the program starts, the list is initialized with this function: The list is declared in my main as List L; and is called like this: Initialize (&L);

void Initialize (List *L) {
  L->size = 0;
  L->head = NULL;
}

From here, I am able to get the list size set to 0 correctly.

I then proceed to add Employee's to the list with these two functions. The first (EmployeeCreation) creates the employee, while the second (Insert) takes it and inserts it into the list. The employee is declared in my main as Employee E; and called like this EmployeeCreation(XXX, &E);

void EmployeeCreation (int idNumber, Employee *E) {   
  E->idNuber = idNumber;
  E->next = NULL;  
}

void Insert (Item X, int position, List *L) {
  int i;
  Item *currentPtr,*previousPtr;
  if(L->head == NULL)
  L->head = &X;
  else{
    currentPtr = L->head;
    for(i=0;i<position;i++){
      previousPtr = currentPtr;
      currentPtr = currentPtr->next;
    }
  previousPtr->next = &X;
  X.next = currentPtr; 
  L->size = L->size + 1;
  }  
}

When run with a test file of 4 ID's. the first is read in correctly and saved to the list. The second, replaces the first, and the length of the list increases by 1 (I understand this part). After this, the program Seg Faults. Where would this error be occurring in my program? Even if the Insert function wants to override a position already occupied I think it should make room just fine and stitch in the new location.

This will not compile - as the data structure Employee does not have a member name. So why give code that actually compiles? — Ed Heal
– Ed Heal, Commented Sep 25, 2013 at 19:40
I have removed that, I deleted some of the code for simplicity. — user2225940
– user2225940, Commented Sep 25, 2013 at 19:45
@user2225940 - There is simplicity but removing code so that it does not even compile is not letting us with a fighting chance. — Ed Heal
– Ed Heal, Commented Sep 25, 2013 at 19:58

David Ranieri · Accepted Answer · 2013-09-25 20:59:43Z

2

You are assigning the address of a local variable (item X) to L->head

L->head = &X;

When you exit from Insert, L->head points to garbage because X is no longer accesible, to fix this you can:

void Insert (Item *X, int position, List *L) {
  ...
  L->head = X;
  ...  
}

Item *temp = malloc(sizeof(Item));

Insert(temp, ..., ...);
...
free(temp);

edited Sep 25, 2013 at 20:59

answered Sep 25, 2013 at 19:42

David Ranieri

41.2k8 gold badges58 silver badges103 bronze badges

Sign up to request clarification or add additional context in comments.

11 Comments

user2225940 Over a year ago

Removing the & will fix this? When this is done I get an error saying incompatible types for the assignment.

David Ranieri Over a year ago

Nops, when you exit from your function item is no longer accesible.

user2225940 Over a year ago

Instead of assigning the address of a local variable how else would I be able to move the information from the employee create to insert?

David Ranieri Over a year ago

You have to pass the address of X to the function and remove the &

user2225940 Over a year ago

Is there no other method than that? Making Item X Item *X is not close to ideal for this situation.

|

Ed Heal · Accepted Answer · 2013-09-25 21:02:45Z

1

Here you go:

void Insert (Item X, int position, List *L) {
  int i;
  Item *currentPtr,*previousPtr;
  Item *newX = malloc(sizeof(Item);
  newX->idNunmber - X.idNumber;
  newX->next = NULL;

  if (L->head == NULL) {
      L->head = newX;
  } else {
    currentPtr = L->head;
    previousPtr = NULL;
    for(i=0;currentPtr!= NULL && i<position;i++) {
      previousPtr = currentPtr;
      currentPtr = currentPtr->next;
    }

    newX->next = currentPtr;
    previousPtr->next = newX; 
    L->size +=1;
  }  
}

edited Sep 25, 2013 at 21:02

answered Sep 25, 2013 at 20:43

Ed Heal

60.3k18 gold badges91 silver badges137 bronze badges

5 Comments

user2225940 Over a year ago

Thanks for all the help, however I am getting a seg fault when I try to add my second employee to the list.

user2225940 Over a year ago

I've been able to determine it crashes at the start of the for loop, any ideas as to why that is?

Ed Heal Over a year ago

How is L initialised?

user2225940 Over a year ago

The initialize function is written above and L is initialized as List L;

user2225940 Over a year ago

I believe you're code is fine, I have error else where in my code that need fixing.

hrv · Accepted Answer · 2013-09-25 19:42:15Z

0

In the Insert function, you could simplify this part and check --

else{
  currentPtr = L->head;
  for(i=0;i<position;i++){
    currentPtr = currentPtr->next;
  }
  Item *nextItm = currentPtr.next;
  currentPtr.next = &X;
  X .next = nextItm ; 
  L->size = L->size + 1;
}

answered Sep 25, 2013 at 19:42

hrv

9556 silver badges14 bronze badges

1 Comment

user2225940 Over a year ago

Simplify in what way? I thought I already had the bar minimum there to make it work.

Collectives™ on Stack Overflow

Creating a singly linked list with 2 structures in C

3 Answers 3

11 Comments

5 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

11 Comments

5 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related