0
/**
 * Definition for singly-linked list.
 * struct ListNode {
 *     int val;
 *     struct ListNode *next;
 * };
 */
struct ListNode* removeElements(struct ListNode* head, int val) {

    struct ListNode *beforeNode = head;

    while(beforeNode != NULL){

        if(head == beforeNode && head->val == val){
            struct ListNode* q = head;
            head = q->next;
            beforeNode = head;
            free(q);
        }
        else if(beforeNode->next != NULL && beforeNode->next->val == val){
            struct ListNode *p = beforeNode->next;
            beforeNode = p->next;
            free(p);
        }

       else
           beforeNode = beforeNode->next;

    }

    return head;
}
7
  • I don't see any problems with this code Commented Apr 4, 2019 at 4:56
  • 2
    What error do you get? Commented Apr 4, 2019 at 4:57
  • can you povide a full compilable piece of code hat causes the problem? Commented Apr 4, 2019 at 4:59
  • no I did not get a problem but leetcode compiler says you used a memory which has been freed, but I don't thinks so. It may be a problem on their part Commented Apr 4, 2019 at 5:07
  • @bake To make sense of that, we'd need enough code to replicate the problem. Commented Apr 4, 2019 at 5:17

1 Answer 1

1

This code works on Xcode but fails in leetcode compiler

Well, the code is not working on any platform.

The test you have done on Xcode must have been incomplete as the code has undefined behavior on any platform.

Look at this simple example:

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

struct ListNode
{
  struct ListNode *next;
  int val;
};

void printList(struct ListNode* p)
{
  if (p)
  {
    printf("%p %d", (void*)p, p->val);
    while(p->next)
    {
      printf("->");
      p = p->next;
      printf("%p %d", (void*)p, p->val);
    }
  }
  printf("\n");
}


// Function from question
struct ListNode* removeElements(struct ListNode* head, int val) {

    struct ListNode *beforeNode = head;

    while(beforeNode != NULL){

        if(head == beforeNode && head->val == val){
            struct ListNode* q = head;
            head = q->next;
            beforeNode = head;
            free(q);
        }
        else if(beforeNode->next != NULL && beforeNode->next->val == val){
            struct ListNode *p = beforeNode->next;
            beforeNode = p->next;
            printf("Free val %p %d\n", (void*)p, p->val);
            free(p);
        }

       else
           beforeNode = beforeNode->next;

    }

    return head;
}

int main()
{
  // Initialize a list with three element like: 1->42->1->NULL
  struct ListNode *head = malloc(sizeof *head);
  head->val = 1;
  head->next = malloc(sizeof *head);
  head->next->val = 42;
  head->next->next = malloc(sizeof *head);
  head->next->next->val = 1;
  head->next->next->next = NULL;

  printList(head);

  removeElements(head, 42);

  printList(head);

  return 0;
}

Example output:

0x558311c02260 1->0x558311c02280 42->0x558311c022a0 1
Free val 0x558311c02280 42
0x558311c02260 1->0x558311c02280 42

As you can see there are two problems:

  • The resulting list is 1->42 but we expected 1->1 In other words - the list is broken.

  • The node printed in the last line is the node that we have just freed (i.e. 0x558311c02280). That is undefined behavior.

The problem is this line:

beforeNode = p->next;

it shall be

beforeNode->next = p->next;

Output after above change:

0x561def3e6260 1->0x561def3e6280 42->0x561def3e62a0 1
Free val 0x561def3e6280 42
0x561def3e6260 1->0x561def3e62a0 1

Now the list is correct and there is no use of freed memory.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.