Repository: valkey-io/valkey
File: src/cluster_legacy.c (C)

Code before:
    NULL,                  /* key dup */
    dictSdsKeyCaseCompare, /* key compare */
    dictSdsDestructor,     /* key destructor */
    NULL,                  /* val destructor */
    NULL                   /* allow to expand */
};

/* Cluster shards hash table, mapping shard id to list of nodes */
dictType clusterSdsToListType = {
    dictSdsHash,        /* hash function */
    NULL,               /* key dup */
    dictSdsKeyCompare,  /* key compare */
    dictSdsDestructor,  /* key destructor */
    dictListDestructor, /* val destructor */
    NULL                /* allow to expand */
};

typedef struct {
    enum { ITER_DICT, ITER_LIST } type;
    union {
        dictIterator di;
        listIter li;
    };
} DictOrListIterator;

void iterInitForDict(DictOrListIterator *iter, dict *d) {
    iter->type = ITER_DICT;
    dictInitSafeIterator(&iter->di, d);
}

void iterInitForList(DictOrListIterator *iter, list *l) {
    iter->type = ITER_LIST;
    listRewind(l, &iter->li);
}

void *iterNext(DictOrListIterator *iter) {
    switch (iter->type) {
    case ITER_DICT: {
        // Get the next entry in the dictionary
        dictEntry *de = dictNext(&iter->di);
        // Return the value associated with the entry, or NULL if no more entries
        return de ? dictGetVal(de) : NULL;
    }
    case ITER_LIST: {
        // Get the next node in the list
        listNode *ln = listNext(&iter->li);
        // Return the value associated with the node, or NULL if no more nodes
        return ln ? listNodeValue(ln) : NULL;
    }
    default: serverPanic("bad type"); return NULL; // This line is unreachable but added to avoid compiler warnings
    }

Review comment (suggestion):
So you decided to take it. :D So, the prefix for the function is "iter", given the struct name is a bit long. (It was not meant to be copied as is, but that's the danger in writing code in review comments I guess.)

If this iterator is only for nodes in this file, then maybe clusterNodeIter? Then and the struct can be a cluster node iter too, and next can return a `clusterNode *` rather than `void *`...?

If want to make it more generic, then its own file and a more generic name would be better, but for now node iterator seems better to me. I don't think we want to iterate a dict OR a list in any other situation.

Code after:
 * we can re-add this node. The goal is to avoid reading a removed
 * node for some time. */
dictType clusterNodesBlackListDictType = {
    dictSdsCaseHash,       /* hash function */
    NULL,                  /* key dup */
    dictSdsKeyCaseCompare, /* key compare */
    dictSdsDestructor,     /* key destructor */
    NULL,                  /* val destructor */
    NULL                   /* allow to expand */
};

/* Cluster shards hash table, mapping shard id to list of nodes */
dictType clusterSdsToListType = {
    dictSdsHash,        /* hash function */
    NULL,               /* key dup */
    dictSdsKeyCompare,  /* key compare */
    dictSdsDestructor,  /* key destructor */
    dictListDestructor, /* val destructor */
    NULL                /* allow to expand */
};

typedef struct {
    enum { ITER_DICT, ITER_LIST } type;
    union {
        dictIterator di;
        listIter li;
    };
} ClusterNodeIterator;

static void clusterNodeIterInitAllNodes(ClusterNodeIterator *iter) {
    iter->type = ITER_DICT;
    dictInitSafeIterator(&iter->di, server.cluster->nodes);
}

static void clusterNodeIterInitMyShard(ClusterNodeIterator *iter) {
    list *nodes = clusterGetNodesInMyShard(server.cluster->myself);
    serverAssert(nodes != NULL);
    iter->type = ITER_LIST;
    listRewind(nodes, &iter->li);
}

static clusterNode *clusterNodeIterNext(ClusterNodeIterator *iter) {
    switch (iter->type) {
    case ITER_DICT: {
        /* Get the next entry in the dictionary */
        dictEntry *de = dictNext(&iter->di);
        /* Return the value associated with the entry, or NULL if no more entries */
        return de ? dictGetVal(de) : NULL;
    }
    case ITER_LIST: {
        /* Get the next node in the list */