0

I need to remove a node from a singly linked list. I know this is a simple thing to do, but my mind is blank and I've searched both Google and Stackoverflow, but I seriously haven't found anything that will help me.

basically the list of nodes is contained in a bucket; like this:

struct node{
  unsigned char id[20];
  struct node *next;
};

struct bucket{
  unsigned char id;
  struct node *nodes;
};

and I have a function

struct bucket *dht_bucketfind(unsigned char *id);  // return bucket with id[20]

to find the correct bucket. So I know how to find the correct bucket, but I don't know how to remove a given node. I would like to remove the node by nodeid (I think, I haven't really written the code that will call the remove function yet ;) but I think I'll be able to modify the code if necessary). I think that's all that's needed to solve this. Thanks in advance.

4
  • If this is homework, add the 'homework' tag to inform potential answerers. Commented Feb 27, 2011 at 22:46
  • hmmm... now that I reread this, I'm thinking your right mkb, ah well. Commented Feb 27, 2011 at 22:55
  • It's not homework. I'm implementing Kademlia as a hobby project. Commented Feb 27, 2011 at 23:39
  • "I don't know how to remove a given node" -- I'm not clear on how that's possible. Draw a picture of your linked list with arrows for pointers, then put an X through a node and redraw the arrows so it is no longer on the list. Then write code that performs that operation, and check that it works for all cases ... e.g., if the node is the first one on the list, or the list is empty. Commented Feb 28, 2011 at 9:04

8 Answers 8

2

If you know the item you want to remove, you must do two things:

  1. Change all pointers that point to the target item to point to the target item's next member. This will be the preceding item's next pointer, or the head of the list bucket.nodes.
  2. Free the node you just made unreachable.

The code for manipulating a linked list is really not that tricky, once you understand what you are doing.

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

Comments

2

Your nodes don't have any payload other than an id, so, depending on the data payload of a node, you might not actually need to iterate the list in the standard way. This is useful if deleters are going to know the address of only the node they want to delete.

If your payload is a pointer to other data:

struct _node {
     void *data;
     unsigned char id[20];
     struct _node *next
}

Then you could "delete" a node by stealing the payload of the next node, and then delinking the next node:

int delete (struct _node *node)
{
     struct _node *temp;

     memcpy(node->id, node->next->id, 20);
     free_function(node->data);
     node->data = node->next->data;

     temp = node->next;
     node->next = node->next->next);
     free(temp);

     return 0;
 }

Comments

1
/* define your two pointers, prev and cur */
prev=NULL;
cur=head;
/* traverse the list until you find your target */
while (cur != NULL && cur->id != search_id) {
  prev=cur;
  cur=cur->next;
}
/* if a result is found */
if (cur != NULL) {
  /* check for the head of the list */
  if (prev == NULL)
    head=cur->next;
  else
    prev->next=cur->next;
  /* release the old memory structure */
  free(cur);
}

2 Comments

...except that you will need to use a comparison function, like strcmp(), to compare the id fields.
yes, and define the two pointers, and change head and search_id to whatever those var names are. I was trying to make this more generic not realizing the submitter was also using id. Thanks for catching that caf.
1
public void Remove(T data)
{
    if (this.Head.Data.Equals(data))
    {
        this.Head = this.Head.Next;
        this.Count = this.Count - 1;
    }
    else
    {
        LinkedListNode<T> node = this.Head;
        bool found = false;
        while (node.Next != null && !found)
        {
            if (node.Next.Data.Equals(data))
            {
                found = true;
                node.Next = node.Next.Next;
                this.Count = Count - 1;
            }
            else
            {
                node = node.Next;
            }
        }
    }
}

Comments

0

Its been a long time ago since I worked with C, but this should be compile clean.

Basically, you need to keep track of the previous pointer while you iterate through the linked list. When you find the node to delete, just change the previous pointer to skip the delete node.

This function deletes all nodes with id (find). If you want to delete only the first occurrence, then put a return after the free statement.

void delete(struct bucket *thisBucket, unsigned char find[20]) {
  struct node *prev = null;
  struct node *curr = thisBucket->nodes;

  while (curr != null) {
    if (!strcmp(curr->id, find)) { // found the node?
      if (prev == null) { // going to delete the first node (header)?
        thisBucket->nodes = curr->next;  // set the new header to the second node
      } else {
        prev->next = curr->next;
      }
      free(curr);

      // if deleted the first node, then current is now the new header,
      // else jump to the next
      curr = prev == null? thisBucket->nodes : prev->next;

    } else { // not found, keep going
      prev = curr;
      curr = curr->next;
    }
  }
}

Comments

0

The following does not contain any error checking and only removes the current node from the list ...

pPrev->next = pCurrent->next;

Your preferences may vary, but I tend to put my linked list node at the start of the structure (whenever practical).

struct node{
  struct node *next;
  unsigned char id[20];
};

struct bucket{
  struct node *nodes;
  unsigned char id;
};

I find this generally helps to simplify pointer arithmetic and allows simple typecasting when needed.

Comments

0

This removes a node given its address; you can modify it to remove a node given its id, but you haven't specified the form of an id -- is it a NUL-terminated string, or is it 20 bytes?

// remove node from bucket and return true
// or return false if node isn't in bucket
int dht_rmnode(struct bucket* bucket, struct node* node)
{
    struct node** ppnode = &bucket->nodes;
    for( ;; ){
        struct node* pnode = *ppnode;
        if( pnode == NULL ) return 0;

        if( pnode == node ){
            *ppnode = pnode->next;
            return 1;
        }
        ppnode = &pnode->next;
    }
}

Or, more compactly,

// remove node from bucket and return true
// or return false if node isn't in bucket
int dht_rmnode(struct bucket* bucket, struct node* node)
{
    struct node** ppnode = &bucket->nodes;
    struct node* pnode;
    for( ; (pnode = *ppnode); ppnode = &pnode->next )
        if( pnode == node ){
            *ppnode = pnode->next;
            return 1;
        }

    return 0;
}

Comments

0
typedef struct node
{
int id;
struct node* next;
}Node;
void delete_element(void)
{
int i;
Node* current=head;
Node* brev=NULL;

if(i==head->id){
head=current->next;
free(current);}
else{
while(NULL!=current->next)
    {
        if(i==current->next->id){
        brev=current;
        current=current->next;
        break;}
        else
        current=current->next;
    }
if(i==current->id)
    {
        if(NULL==head->next){
        head=NULL;
        free(current);}
        else
        brev->next=current->next;
        free(current);
    }
else
    printf("this id does not exist\n");
}
}

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.