Delete unreachable code in _quicklistInsert()

[LiiNen]

This PR might be same issue with #12613
The point is that in void _quicklistInsert(), the condition if (!node) { ... } is unreachable.

Analysis

Following the function calls, the only two use _quicklistInsert() like below:

• LINSERT key <BEFORE | AFTER> pivot element
• RM_ListInsert() from module.c
• quicklistTest() from quicklist.c

LINSERT

During checking next node, if the current node is NULL, then it just return 0, so that we cannot reach it.

void linsertCommand(client *c) {
    // ...
    iter = listTypeInitIterator(subject,0,LIST_TAIL);
    while (listTypeNext(iter,&entry)) {  // check entry exists
        if (listTypeEqual(&entry,c->argv[3])) {
            listTypeInsert(&entry,c->argv[4],where);
            inserted = 1;
            break;
        }
    }
    // ...
}

int listTypeNext(listTypeIterator *li, listTypeEntry *entry) {
    // ...
    if (li->encoding == OBJ_ENCODING_QUICKLIST) {  // if quicklist, check next
        return quicklistNext(li->iter, &entry->entry);
    }
    // ...
}

int quicklistNext(quicklistIter *iter, quicklistEntry *entry) {
    // ...
    entry->node = iter->current;

    if (!iter->current) {  // if entry node is NULL, return 0, so that listTypeInsert() cannot be executed
        D("Returning because current node is NULL"); 
        return 0;
    }
    // ...

RM_ListInsert()

Similar with linsertCommand(), iterator already check if key exists or pivot exists,
so that we will not encounter if (!node) { ... }.

int moduleListIteratorSeek(RedisModuleKey *key, long index, int mode) {
    if (!key) {
        errno = EINVAL;
        return 0;
    } else if (!key->value || key->value->type != OBJ_LIST) {
        errno = ENOTSUP;
        return 0;
    } if (!(key->mode & mode)) {
        errno = EBADF;
        return 0;
    }

    long length = listTypeLength(key->value);
    if (index < -length || index >= length) {
        errno = EDOM; /* Invalid index */
        return 0;
    }
    // ...

quicklistTest()

This is for test and callable from other files.
This function calls quicklistInsertBefore() and quicklistInsertAfter() directly, but always following after quicklistGetIteratorEntryAtIdx() or quicklistNext(), so if (!node) { ... } is unreachable.

what I have done

I'm finally sure that if (!node) { ... } from _quicklistInsert() is unreachable, so I have deleted it.
If I missed something, then please let me know.
Thanks a lot.

[sundb]

related to #12613

[LiiNen]

Oh, I missed one.
Only function quicklistInsertAfter(), not quicklistInsertBefore(), called from quicklistReplaceEntery().
But this case also has condition with entry->node, so that it cannot be NULL.

void quicklistReplaceEntry(quicklistIter *iter, quicklistEntry *entry,
                           void *data, size_t sz)
{
    ...
    } else if (QL_NODE_IS_PLAIN(entry->node)) {
        if (isLargeElement(sz, quicklist->fill)) { ... }
        else {
            quicklistInsertAfter(iter, entry, data, sz);
            __quicklistDelNode(quicklist, entry->node);
        }
    }
    ...
}

[CLAassistant]

All committers have signed the CLA.