0

Why is my C code running an infinite loop and how do I fix it to obtain my expected output ?

I am supposed to reverse nodes from A to B

So let's say I give an input of :

2 5

1 2 3 4 5 6 7

The first line is to indicate from which index to which index I am supposed to reverse the nodes So here 3 is node A and 6 is node B. The second line is the list I have given. So, I am expecting an output of : 1 2 6 5 4 3 7

This is my code :

ListNode *reverseSegment(ListNode *head, int start, int end)
{
  // Write your code here

  // Condition to Check if B is valid (if B is valid, then A is valid)
  ListNode *temp1 = head;
  int count = 0;
  while (temp1 != NULL)
  {
    temp1 = temp1->next;
    count++;
  }

  // Condtion to check if index out of range or invalid
  if (count < end)
  {
    return head;
  }

  // Declaring Pointers
  ListNode *prev = NULL, *cur = head, *next = NULL, *endptr = NULL;
  ListNode *temp2 = head; 
  int diff = end - start;

  // Point endptr to the last node of the list
  while (temp2->next != NULL)
  {
    endptr = temp2; 
  }
  // Point next ptr to the node after A
  // Point prev pointer to the node after B
  //next = cur->next;
  prev = endptr;
  
  // Reversing the node from B to A
  while ((diff+1) > 0 )
  {
    next = cur->next;
    cur->next = prev;
    prev = cur;
    cur = next;
    diff--;
  }
  // Point head ptr to node B
  head->next = prev;

  //return head;
} 
Turamarth
  • 2,282
  • 4
  • 25
  • 31
  • Welcome to Stack Overflow. Have you considered the possibility that your linked list nodes, prior to calling the function, are not actually properly connected into a linked list, but instead form a cycle? Have you tried to [debug](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) the code, in order to figure out *which* loop is running infinitely? – Karl Knechtel Feb 02 '22 at 15:25
  • 1
    If you actually check the indices being correct you should check for start being smaller than end as well... – Aconcagua Feb 02 '22 at 15:25
  • 1
    `while (temp2->next != NULL)` in this loop you are not moving the `temp2` to the next node, and if `temp2->next` is not a `nullptr` then the loop will be `while(true)` so that will become an infinite loop. You should add this line at the end `temp2 = temp2->next` in that while loop. – iwrestledthebeartwice Feb 02 '22 at 15:35
  • Please, edit your question to include a [Minimal, Verifiable and Complete Example](https://stackoverflow.com/help/minimal-reproducible-example) so we can test your code and verify it behaves as you describe. This will result in a better support from our side. – Luis Colorado Feb 03 '22 at 09:34

2 Answers2

0

This while loop will be infinite since temp2 is never modified in each loop:

  // Point endptr to the last node of the list
  while (temp2->next != NULL)
  {
    endptr = temp2; 
  }

It looks like you really need to search for the node before A and B, so something like this may be simpler solution (for start > 0):

  int      count = 0;
  ListNode *cur = head;

  ListNode *start_node_prev;
  ListNode *start_node;
  ListNode *start_node_next;
  ListNode *end_node_prev;
  ListNode *end_node;
  ListNode *end_node_next;

  while(count < start)
  {
     start_node_prev = cur;
     cur             = cur->next;
     start_node      = cur;
     start_node_next = cur->next;
  }

  while(count < end)
  {
     end_node_prev = cur;
     cur           = cur->next;
     end_node      = cur;
     end_node_next = cur->next;
  }

  start_node->next = end_node_next;
  end_node->next   = start_node_next;

  start_node_prev->next = end_node;
  end_node_prev->next   = start_node;

Note that a separate case is needed if start == 0.

Also, a check should be include for start < end.

Jonathon S.
  • 1,928
  • 1
  • 12
  • 18
0

You are doing stuff a bit too complex, especially you are iterating around too much.

At very first you might want to check, if start indeed is smaller than end as well! By the way: Negative indices are pretty meaningless, so I'd recommend unsigned types to specify them, e.g. size_t.

You are now interested in the offset between start and end, so I'd calculate that one first:

end -= start;

You won't need the original value any more, see below.

Then you iterate from head onward:

if(!head || start == end) // empty list or sequence
{
    return head;
}

ListNode* startNode = head;
for(;start; --start)
{
    if(!startNode->next)
    {
       // no nodes left -> start is not valid!
       return head;
    }
    startNode = startNode->next;
}
ListNode* endNode = startNode;
for(;end; --end)
{
    if(!endNode->next)
    {
       // no nodes left -> end is not valid!
       return head;
    }
    endNode = endNode->next;
}

Now both start and end are down-counted to 0, but you don't need them any more anyway – you found both start and end node, and from now on you can operate on the nodes directly.

Within the list segment you now can reverse by simply swapping the prev and next pointers for every node:

for(ListNode* cur = endNode; cur != startNode;  cur = cur->next)
// note that you swapped pointers already when
// getting here so you still iterate backwards: ^^^^^^^^^^^^^^^
{
    ListNode* tmp = cur->next;
    cur->next = cur->prev;
    cur->prev = tmp;
}

Now left to fix: endNode's predecessor points to what has been its successor, which, though, needs to get the successor of the start node, whereas the start node hasn't been changed at all yet. So we need:

ListNode* tmp = endNode->prev;
endNode->prev = startNode->prev;   // tail now fixed completely
startNode->prev = startNode->next; // swap direction
startNode->next = tmp;             // set to the remaining list

Finally:

return endNode->prev ? head : endNode;

If startNode has a predecessor then we reversed from somewhere in the middle onward, thus need to return head – otherwise we reversed from the beginning of the list and original head node has been moved away, while the original end node now is the new head.

Be aware that above is untested code – if you discover a bug, please fix yourself...

Aconcagua
  • 24,880
  • 4
  • 34
  • 59