Reputation: 1
I am fairly new to c (and this site) and I am having a lot of issues with segmentation faults. I am writing a program that creates a linked list of numbers and inserts values in ascending order.
void insert(struct element **head, struct element *new){
if((*head)->next == NULL && (*new).i > (*(*head)->next).i){
(*head)->next = new;
return;
}
if((*head)->next == NULL && (*new).i < (*(*head)->next).i){
new->next = (*head)->next;
*head = new;
return;
}
struct element *prev = *head;
struct element *current = (*head)->next;
while(current->next != NULL){
if((*new).i < (*current).i){
prev = current;
current = current->next;
} else if((*new).i > (*current).i){
new->next = current;
prev->next = new;
}
}
}
int main (void){
struct element **head;
int value;
printf("%s", "TEST" );
printf("%s" , "Please type in an integer value. ");
scanf("%d" , &value);
printf("%s", "TEST" );
do{
printf("%s", "TEST" );
struct element *new;
if((new = malloc(sizeof(struct element))) == NULL){
return(NULL);
}
printf("%s", "TEST" );
(*new).i = value;
printf("%s", "TEST" );
if(head == NULL){
*head = new;
printList(*head);
} else if(value <= 0){
printListBackwards(*head);
}
else {
insert(head, new);
printList(*head);
}
} while(value > 0);
I don't need help on whether the logic is correct with inserting or anything. I haven't even had a chance to really test it because I immediately get a segmentation fault after I enter an integer after the prompt. I know it seems funky, but the specs call for you to use a pointer to a pointer to a structure (the head of the linked list).
Upvotes: 0
Views: 874
Reputation: 4528
There is a seg fault being caused on the second line of your post
if((*head)->next == NULL && (*new).i > (*(*head)->next).i){
(*head)->next = new;
return;
}
Segmentation fault means that you are trying to access memory that you are not allowed to. For example, you can't dereference a NULL pointer.
Your if
statement is evaluated like this.
Check that (*head)->next
is null.
if it is not NULL, then skip the rest.
if it IS NULL, then you can replace each following (*head)->next
with NULL
. This means that the following part && (*new).i > (*(*head)->next.i)
can be rewritten as follows && (*new).i > ((*NULL).i)
...
In short, you are trying to dereference a NULL pointer value.
Please also refer to @Parker Kemp's post. There is a number of times that you are checking for NULL correctly but misinterpreting it's meaning.
I could rewrite the code for you but I think that you will benefit more from going through a tutorial like this one or this one
I strongly recommend drawing a diagram of your data structure and drawing arrows for pointers.
Upvotes: 0
Reputation: 16406
struct element **head;
You don't want that. Instead,
struct element *head = NULL;
Then, when you call insert, use
insert(&head, new);
You have many other bugs and poor usages, but that's a start for your specific problem.
Upvotes: 0
Reputation: 765
Are you sure you want head to be an element**
and not an element*
? That extra degree of separation is causing you problems, not the least of which is very difficult-to-read code.
Here's the main thing that jumps out at me:
if(head == NULL){
*head = new;
printList(*head);
}
You're confirming that head is a NULL pointer, and then immediately trying to dereference it with *
. If you really insist on head being a double-pointer, then you need to dynamically allocate it before dereferencing it. Like so:
if(head == NULL){
head = malloc(sizeof(element*));
*head = new;
printList(*head);
}
That actually may not be syntactically perfect (I come from C++), but you get the idea. Speaking of C++ though, it's usually considered bad practice to name a variable "new" in C, because new
is a keyword in C++.
Upvotes: 2