CODEWITHSUNDEEP
csegmentation-fault
user2247155
user2247155

Reputation: 19

I have a segmentation error in C

I have a Segmentation error, maybe a lot more after I run it, but I can't check anything else now because of that.

The program should work like this:

This is my code so far:

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

struct element {
int i;
struct element *next;
};

void insert (struct element **head, struct element *new)
{

struct element *temp;
temp = *head;

while(temp->next != NULL)
{
    if((*head==NULL))
    {
    head = malloc(sizeof(struct element));
    //temp->i = i;
    temp->next = new;
    new = temp;
    }
    else if(temp->i == new->i)
    {
        new = malloc(sizeof(struct element));
        free(new);
        //purge(&head,&new);

    }
    else if(temp->i < new->i)
    {
        temp->i = new->i;
    }
    else if(temp->i > new->i)
    {
        new = new->next;
    }
}

}

void purge (struct element *current, struct element *predecessor)
{

predecessor->next = current -> next;
free(current);
}

void printList (struct element *head) 
{
while(head)
{
    printf("%d", head -> i);
    head = head->next;
}

}

void printListBackwards (struct element *ptr)
{
if(ptr == NULL)
{
    printf("list is empty \n");
    return;
}
if(ptr->next != NULL)
{
    printListBackwards(ptr->next);
}
printf("print %p %p %d\n", ptr, ptr->next, ptr->i);
}


int main()
{
int n = 0;
int count = 5;
printf("enter a Number: \n");
scanf("%d",&n);
struct element *new;
new = malloc(sizeof(struct element));
struct element *head = NULL;
new->i = n;
while(count!=0)
{
    insert(&head,new);
    printList(head);    
    count++;
}

}

Upvotes: 0

Views: 93

Answers (3)

Arun
Arun

Reputation: 20383

Do you really need a linked list? It seems the problem statement says that user can enter only 5 numbers... if so, why not just use an array of 5 elements? Following are some ideas.

enum { N = 5 };
typedef struct Element {
    int number;
    bool present;
} Element;

Element elements[ N ];

Init:
for( i = 0; i != N; ++i ) {
    elements[i].number = 0;
    elements[i].present = false;
}

Insert "inputNumber":
for( i = 0; i != N; ++i ) {
    if( elements[i].present == false ) {
        elements[i].number = inputNumber;
        elements[i].present = true;
    }
}

Remove "removeNumber":
for( i = 0; i != N; ++i ) {
    if( elements[i].number == removeNumber ) {
        elements[i].present = false;
    }
}

Print Backwards:
for( i = N; i != 0; --i ) {
    printf( "%d\n", elements[i].number );
}

Upvotes: 1

Jonathan Leffler
Jonathan Leffler

Reputation: 754090

In the main() function, you only allocate and create one element with malloc(); you then try to add it to your list 5 times. This is going to cause confusion. You should allocate a node once for each element you add to the list.

struct element *head = NULL;

while (count!=0)
{
    printf("enter a Number: \n");
    if (scanf("%d", &n) != 1)
        break;
    struct element *new = malloc(sizeof(struct element));
    if (new == 0)
        break;
    new->i = n;
    new->next = NULL;
    insert(&head, new);
    printList(head);    
    count--;
}

Note that the revised code checks the result of both scanf() and malloc(). It also sets the new element's next pointer to NULL. And it counts down rather than up; this is likely to use less memory.

I've not tested this so there could be (and very probably are) other problems, but this is likely to work better (fix some of the problems, but not all of the problems).

You do need to learn how to use a debugger, at least enough to get the stack trace so you know which line of code is causing the crash.

Upvotes: 1

Mats Petersson
Mats Petersson

Reputation: 129374

In main, you should set new->next = NULL; [or somewhere in the beginning of insert]

This bit of code is just messed up:

head = malloc(sizeof(struct element));
//temp->i = i;
temp->next = new;
new = temp;

You should, probably, set 

*head = new;

But you also need to set *head->next = NULL;

This bit is complete nonsense:

    new = malloc(sizeof(struct element));
    free(new);
    //purge(&head,&new);

You would want to free new. 

else if(temp->i < new->i)
{
    temp->i = new->i;
}
else if(temp->i > new->i)
{
    new = new->next;
}

This is also quite wrong. I think the last one should do

   temp = temp->next;

Do yourself a favour, draw up on a piece of paper, boxes 

  HEAD 
   !
   v
 +-----+
 ! i=3 !
 +-----+     +-----+
    !------->! i=4 !
             +-----+
                !-------->NULL

And then walk through it and see how your code inserts, removes, etc.

[Can I also suggest that you don't use C++ reserved words in your code - new is a C++ reserved word. It means that your code CERTAINLY won't compile in a C++ compiler, which is a bad thing to prevent. Sure, there are several other things that may need changing, but a simple thing like "not calling a variable new" shouldn't be one of the things it fails on].

Upvotes: 0

Related Questions