C How to insert a new node after an existing node in a Linked List?-CodePudding

int add_after(ITEM *list, ITEM *c_item, int value)
{
    //create a new node   
    ITEM *elem = malloc(sizeof(ITEM));
    elem->value = value;

    //if head is NULL, it is an empty list
    if (elem == NULL || c_item == NULL || list == NULL) {
        return -1;
    } else {
        while (list != NULL) {
            if (list == c_item) {
                elem->next = list->next;
                elem->value = value;
                list->next = elem;
                list = list->next;
            }
            list = list->next;
        }
    }
    return 0;
}

I am trying to make a function the puts a node after an existing node. my problem is inside the else statement. I am trying to find a node list equal to node c_item. After finding the equal, it should enter elem in between them.

CodePudding user response：

I would rewrite your function as follow:

int add_after(ITEM *list, ITEM *c_item, int value)
{
    //create a new node
    ITEM *elem;

    //if head is NULL, it is an empty list
    if (!list || !c_item) {
        return -1;
    }

    /* Create a new node. */
    elem = malloc(sizeof(ITEM));
    if (!elem) {
        return -1;
    }

    /* You should rather set the value after having checked the allocated
     * memory is not NULL. */
    elem->value = value;

    /* Insert elem ahead of the current item. */
    elem->next = c_item->next;
    c_item->next = elem;

    return 0;
}

CodePudding user response：

If you wish to insert a new node after a given node, you do not need to iterate on the list, just insert the new node as the next node of the c_item node:

int add_after(ITEM *list, ITEM *c_item, int value) {
    //check function arguments   
    if (list == NULL || c_item == NULL)
        return -1;
    //create a new node   
    ITEM *elem = malloc(sizeof(ITEM));
    if (elem == NULL)
        return -1;
    elem->value = value;
    elem->next = c_item->next;
    c_item->next = elem;
    return 0;
}

In your approach, you try and locate the element in the list, which might be necessary if the caller cannot be trusted. In this case, you should have a different return value whether the element was inserted or not and free the allocated elem if c_item was not found:

int add_after(ITEM *list, ITEM *c_item, int value) {
    //create a new node   
    ITEM *elem = malloc(sizeof(ITEM));

    //if head is NULL, it is an empty list
    if (elem == NULL || c_item == NULL || list == NULL) {
        return -1;
    } else {
        while (list != NULL) {
            if (list == c_item) {
                elem->value = value;
                elem->next = list->next;
                list->next = elem;
                return 1;   // element was inserted
            }
            list = list->next;
        }
        free(elem);
        return 0;   // c_item was not found, elem not inserted
    }
}