-2

everytime I run this loop , I get a segmentation fault in the second iteration of the loop . 

node *new,*new1;

new=(node*) malloc(sizeof(node));
new1=(node*) malloc(sizeof(node));
new = start->next;

for(;new->next != NULL;new = new->next)
{
    for(new1=new->next;new1 != NULL;new1=new1->next)
    {   //printf("LOOP\n");
        if(new->data > new1->data)   
        {
            printf("\n Swapping - new:%d and  new1:%d\n",new->data,new1->data); 
            temp = (node*) malloc(sizeof(node));
            temp1 = (node*) malloc(sizeof(node));
            temp1 = new->next;
            temp = new1->next;
            new->next = temp;
            printf("Temp var : %d\n",temp->data);
            printf("Temp1 var : %d\n",temp1->data);
            new1->next = new;

            new1 = new;
            new = temp1;
            printf("After swapping , new:%d and new1 : %d\n",new->data,new1->data);
            //        free(temp);
            //       free(temp1);
        }
    }
}

Whenever I give a list to it - eg. 4,1,9,6 it only swaps 4 and 1 , when it is the iteration to swap 9 and 6 , it shows and segmentation fault .

CiaPan
  • 9,381
  • 2
  • 21
  • 35
Aditya
  • 1
  • 1
  • 4
    I recommend you take some time to read [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) by Eric Lippert, and learn how to use a debugger to catch crashes like this. That will at least help you locate where in your code the crash happens. The next step is to step through your code, line by line, in a debugger to figure out *why* it might happen. Most likely there's a pointer you don't update correctly, or doesn't initialize at all. – Some programmer dude Oct 20 '17 at 06:44
  • Probably unrelated, but you leak memory (the two `malloc`s for `temp` and `temp1`) – UnholySheep Oct 20 '17 at 06:44
  • 2
    Talking about pointers, `new = start->next;` will lead to a *memory leak*. You first make `new` point to some memory, then you make it point to some *other* memory, losing the original pointer. I also recommend you read [this old question and its answers](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc), about casting the result of `malloc`. – Some programmer dude Oct 20 '17 at 06:45
  • 1
    Your sort routine should not allocate anything, so the code you have now just makes zero sense and should be scrapped immediately. – n. m. could be an AI Oct 20 '17 at 06:46
  • 1
    An observation: The two calls to `malloc` inside the `if` statement, where `temp` and `temp1` are assigned, are nothing more than memory leaks, since immediately after allocating the storage, you immediately assign other values to `temp` and `temp1`, permanently losing that storage without ever having used it. It makes no sense at all. – Tom Karzes Oct 20 '17 at 06:46
  • Lastly, you say you get "a segmentation fault in the second iteration of *the loop*"... *Which* loop? You have two loops, which one is on its second iteration when the crash happens? – Some programmer dude Oct 20 '17 at 06:46
  • Please show a [mcve]. – Jabberwocky Oct 20 '17 at 06:48
  • What are start and start-next initialized to? – jwdonahue Oct 20 '17 at 06:49

1 Answers1

0

After executing temp = new1->next;, temp could be NULL if new1 was the last node of the list. While temp is NULL, it would raise a segment error when you visit it's field in this line printf("Temp var : %d\n",temp->data);

a.l.
  • 1,085
  • 12
  • 29