Compiler shows abort called

Question

I'm trying to delete a node in a singly linked list in a given position.When i submit this code all the test cases are success.But except one and the compiler shows abort called.When i googled it it shows resource exceeded.Is there is any other way to optimize this code to reduce the resource usage. I have written my code inside SinglyLinkedListNode* deleteNode(SinglyLinkedListNode* head, int position) function.

You’re given the pointer to the head node of a linked list and the position of a node to delete. Delete the node at the given position and return the head node. A position of 0 indicates head, a position of 1 indicates one node away from the head and so on. The list may become empty after you delete the node.

Input Format

You have to complete the deleteNode(SinglyLinkedListNode* llist, int position) method which takes two arguments - the head of the linked list and the position of the node to delete. You should NOT read any input from stdin/console. position will always be at least 0 and less than the number of the elements in the list.

The first line of input contains an integer , denoting the number of elements in the linked list. The next lines contain an integer each in a new line, denoting the elements of the linked list in the order. The last line contains an integer

denoting the position of the node that has to be deleted form the linked list.

Constraints

, where is the element of the linked list.

Output Format Delete the node at the given position and return the head of the updated linked list. Do NOT print anything to stdout/console.

The code in the editor will print the updated linked list in a single line separated by spaces.

  #include <bits/stdc++.h>

using namespace std;

class SinglyLinkedListNode {
    public:
        int data;
        SinglyLinkedListNode *next;

        SinglyLinkedListNode(int node_data) {
            this->data = node_data;
            this->next = nullptr;
        }
};

class SinglyLinkedList {
    public:
        SinglyLinkedListNode *head;
        SinglyLinkedListNode *tail;

        SinglyLinkedList() {
            this->head = nullptr;
            this->tail = nullptr;
        }

        void insert_node(int node_data) {
            SinglyLinkedListNode* node = new SinglyLinkedListNode(node_data);

            if (!this->head) {
                this->head = node;
            } else {
                this->tail->next = node;
            }

            this->tail = node;
        }
};

void print_singly_linked_list(SinglyLinkedListNode* node, string sep, ofstream& fout) {
    while (node) {
        fout << node->data;

        node = node->next;

        if (node) {
            fout << sep;
        }
    }
}

void free_singly_linked_list(SinglyLinkedListNode* node) {
    while (node) {
        SinglyLinkedListNode* temp = node;
        node = node->next;

        free(temp);
    }
}

// Complete the deleteNode function below.

/*
 * For your reference:
 *
 * SinglyLinkedListNode {
 *     int data;
 *     SinglyLinkedListNode* next;
 * };
 *
 */
SinglyLinkedListNode* deleteNode(SinglyLinkedListNode* head, int position) {
   int i = 0;

   SinglyLinkedListNode* temp = new SinglyLinkedListNode(2);
    SinglyLinkedListNode* c;
    SinglyLinkedListNode* p;
    c = head;
    p = head;

    for(;i!=position;p=c,c=c->next,i++);
    p->next = c->next;

    delete c;
    
    return head;


   

 


}

int main()

This belongs in [CodeReview](https://codereview.stackexchange.com/). — Mansoor, Jul 22 '20 at 14:22
Please consider a debugger rather than guessing at vague process signals. Resource usage is not necessarily the correct explanation here. Furthermore, please see https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h for some additional best-practices that stand out. — nanofarad, Jul 22 '20 at 14:22
You should be pairing new with delete, not free. The function free_singly_linked_list should be the destructor, not a free function. All the functions except print_singly_linked_list should be in the class. There's a lot of places this code could have gone rogue on you. You need to a good book, and a debugger. — sweenish, Jul 22 '20 at 14:30
@ChungHuang Sounds like a competitive coding site. It's typically some version of g++. Their issue is a ton of memory leaks; deleteNode function is a mess. I mean, the for loop statement has a semi-colon at the end, so the loop is a no-op. `temp` leaks every time the function is called. `temp` isn't even used, so it's just leaking memory for fun. The list is broken every time the function is called since there is no pointer reassignment. Competitive coding sites for learning are trash. — sweenish, Jul 22 '20 at 14:41
`deleteNode` returns a dangling pointer when `position` is zero. It will also fail if the list has fewer than `position` elements. — molbdnilo, Jul 22 '20 at 14:42
@molbdnilo According to the question the list will never be empty and the position value will always be within the range — Stark, Jul 22 '20 at 14:45
@Stark Please include the actual problem specification (indexing in C++ is usually zero-based). It's very difficult to say what you did wrong without knowing what would be right. — molbdnilo, Jul 22 '20 at 15:18
One obvious problem: you allocate things with `new` and then deallocate them with `free`. — user253751, Jul 22 '20 at 15:19
@Stark Well, you *do* have that problem with deleting at position 0 that I mentioned earlier... — molbdnilo, Jul 22 '20 at 15:36
No I don’t have a problem while deleting a node at beginning — Stark, Jul 22 '20 at 15:37
@Stark you have problems deleting, generally. It's a bad function, and it leaks memory all over the place. See my second comment. Given that head is never re-assigned, I'd say you most definitely have a problem deleting the first node. — sweenish, Jul 22 '20 at 16:09
I have removed temp declaration but it still shows abort called only for a particular test case — Stark, Jul 22 '20 at 16:12
@Stark That was far from the only problem I noted. The whole function is bad and still leaks memory. Just less obviously now that you've removed a useless variable. Just for fun, why not drop a link to the problem. I'm curious. — sweenish, Jul 22 '20 at 16:15
The problem statement is potentially contradictory. Are you supposed to be reading from a file? — sweenish, Jul 22 '20 at 16:23
Yeah I think because except the delete node function all other are template code which is present already — Stark, Jul 22 '20 at 16:26
If `position` is 0, you never enter the loop, which leaves `p` and `c` equal to `head`, and the function is equivalent to `head->next = head->next; delete head; return head;`. — molbdnilo, Jul 22 '20 at 17:30
@molbdnilo Yeah deletion at 0 position is the problem so i modified the answer and now it works fine.Thank you all for your response. — Stark, Jul 23 '20 at 04:33

score 0 · Accepted Answer · answered Jul 23 '20 at 04:31

I modified the answer and found that deleteion at position 0 is the problem.Now it works fine.

SinglyLinkedListNode* deleteNode(SinglyLinkedListNode* head, int position) {
       SinglyLinkedListNode* p;
       SinglyLinkedListNode* c;

       p=head;
       c=head;

      if(position==0)
      {
          SinglyLinkedListNode *temp;
          temp=head;
          head = head->next;
          delete temp;
          return head;
      }

       int i = 0;

       for(;i!=position;i++,p=c,c=c->next);

       p->next = c->next;
       delete c;

       return head;     
}

Compiler shows abort called

1 Answers1