tweak dllist to allow deletion mid-iteration

Eventually comms will use this thing I think

xwords4/common/comms.c

@@ -35,6 +35,7 @@
 #include "knownplyr.h"
 #include "device.h"
 #include "nli.h"
+#include "dllist.h"
 
 #define HEARTBEAT_NONE 0
 
@@ -272,7 +273,6 @@ static void sendConnect( CommsCtxt* comms, XWEnv xwe
 #endif
                          );
 
-typedef enum {FEA_OK = 0x00, FEA_REMOVE = 0x01, FEA_EXIT = 0x02} ForEachAct;
 typedef ForEachAct (*EachMsgProc)( MsgQueueElem* elem, void* closure );
 static void forEachElem( CommsCtxt* comms, EachMsgProc proc, void* closure );
 

xwords4/common/dllist.c

@@ -21,38 +21,30 @@
 #include "dllist.h"
 #include "dutil.h"              /* for NULL ??? */
 
-void
-dll_map( DLHead* list, DLMapProc proc, void* closure )
-{
-    for ( ; !!list; list = list->next ) {
-        (*proc)( list, closure );
-    }
-}
-
 DLHead*
 dll_insert( DLHead* head, DLHead* node, DLCompProc proc )
 {
     DLHead* next;
     DLHead* prev = NULL;
-    for ( next = head; !!next; next = next->next ) {
+    for ( next = head; !!next; next = next->_next ) {
         if ( 0 <= (*proc)( next, node ) ) {
             break;
         }
         prev = next;
     }
 
-    node->prev = prev;
+    node->_prev = prev;
-    node->next = next;
+    node->_next = next;
 
     DLHead* newHead;
     if ( !!prev ) {
         newHead = head;
-        prev->next = node;
+        prev->_next = node;
     } else {
         newHead = node;
     }
     if ( !!next ) {
-        next->prev = node;
+        next->_prev = node;
     }
     XP_ASSERT( !!newHead );
     return newHead;
@@ -61,23 +53,61 @@ dll_insert( DLHead* head, DLHead* node, DLCompProc proc )
 DLHead*
 dll_remove( DLHead* list, DLHead* node )
 {
+    XP_ASSERT( !list->_prev );
     DLHead* newHead = list;
     if ( list == node ) {
-        newHead = list->next;
+        newHead = list->_next;
         if ( !!newHead ) {
-            newHead->prev = NULL;
+            newHead->_prev = NULL;
         }
     } else {
-        if ( !!node->prev ) {
+        if ( !!node->_prev ) {
-            node->prev->next = node->next;
+            node->_prev->_next = node->_next;
         }
-        if ( !!node->next ) {
+        if ( !!node->_next ) {
-            node->next->prev = node->prev;
+            node->_next->_prev = node->_prev;
         }
     }
     return newHead;
 }
 
+DLHead*
+dll_map( DLHead* list, DLMapProc mapProc, DLDisposeProc dispProc,
+         void* closure )
+{
+    DLHead* newHead = list;
+    while ( !!list ) {
+        DLHead* next = list->_next;
+        ForEachAct fea = (*mapProc)( list, closure );
+        if ( 0 != (FEA_REMOVE & fea) ) {
+            DLHead* victim = list;
+            next = victim->_prev;
+
+            if ( victim == newHead ) {
+                newHead = next;
+            }
+
+            if ( !!victim->_prev ) {
+                victim->_prev->_next = victim->_next;
+            }
+            if ( !!victim->_next ) {
+                victim->_next->_prev = victim->_prev;
+            }
+
+            if ( !!dispProc ) {
+                (*dispProc)( list, closure );
+            }
+        }
+        if ( 0 != (FEA_EXIT & fea) ) {
+            goto done;
+        }
+
+        list = next;
+    }
+ done:
+    return newHead;
+}
+
 DLHead*
 dll_sort( DLHead* list, DLCompProc proc )
 {

xwords4/common/dllist.h

@@ -25,18 +25,22 @@ extern "C" {
 #endif
 
 typedef struct DLHead {
-    struct DLHead* next;
+    struct DLHead* _next;
-    struct DLHead* prev;
+    struct DLHead* _prev;
 } DLHead;
 
+typedef enum {FEA_OK = 0x00, FEA_REMOVE = 0x01, FEA_EXIT = 0x02} ForEachAct;
+
 typedef int (*DLCompProc)(const DLHead* dl1, const DLHead* dl2);
 
 DLHead* dll_insert( DLHead* list, DLHead* node, DLCompProc proc );
 DLHead* dll_remove( DLHead* list, DLHead* node );
 DLHead* dll_sort( DLHead* list, DLCompProc proc );
 
-typedef void (*DLMapProc)(const DLHead* dl1, void* closure);
+typedef ForEachAct (*DLMapProc)(const DLHead* dl1, void* closure);
-void dll_map( DLHead* list, DLMapProc proc, void* closure );
+typedef void (*DLDisposeProc)(DLHead* elem, void* closure);
+DLHead* dll_map( DLHead* list, DLMapProc mapProc, DLDisposeProc dispProc,
+                 void* closure );
 
 #ifdef CPLUS
 }

xwords4/common/knownplyr.c

@@ -88,15 +88,15 @@ loadState( XW_DUtilCtxt* dutil, XWEnv xwe )
     return state;
 }
 
-static void
+static ForEachAct
 saveProc( const DLHead* dl, void* closure )
 {
     XWStreamCtxt* stream = (XWStreamCtxt*)closure;
     KnownPlayer* kp = (KnownPlayer*)dl;
     stream_putU32( stream, kp->newestMod );
-    XP_LOGFF( "wrote newestMod: %d for player %s", kp->newestMod, kp->name );
     stringToStream( stream, kp->name );
     addrToStream( stream, &kp->addr );
+    return FEA_OK;
 }
 
 static void
@@ -108,7 +108,7 @@ saveState( XW_DUtilCtxt* dutil, XWEnv xwe, KPState* state )
         stream_setVersion( stream, CUR_STREAM_VERS );
         stream_putU8( stream, CUR_STREAM_VERS );
 
-        dll_map( &state->players->links, saveProc, stream );
+        dll_map( &state->players->links, saveProc, NULL, stream );
 
         const XP_UCHAR* keys[] = { KNOWN_PLAYERS_KEY, NULL };
         dutil_storeStream( dutil, xwe, keys, stream );
@@ -198,41 +198,57 @@ compByDate(const DLHead* dl1, const DLHead* dl2)
  *
  * For early testing, however, just make a new name.
  */
+
+typedef struct _AddData {
+    const XP_UCHAR* name;
+    const CommsAddrRec* addr;
+    KnownPlayer* withSameDevID;
+    KnownPlayer* withSameName;
+} AddData;
+
+static ForEachAct
+addProc( const DLHead* dl, void* closure )
+{
+    AddData* adp = (AddData*)closure;
+    KnownPlayer* kp = (KnownPlayer*)dl;
+    if ( 0 == XP_STRCMP( kp->name, adp->name ) ) {
+        adp->withSameName = kp;
+    }
+    if ( adp->addr->u.mqtt.devID == kp->addr.u.mqtt.devID ) {
+        adp->withSameDevID = kp;
+    }
+    ForEachAct result = !!adp->withSameName && !!adp->withSameDevID
+        ? FEA_EXIT : FEA_OK;
+    return result;
+}
+
 static void
-addPlayer( XW_DUtilCtxt* XP_UNUSED_DBG(dutil), KPState* state, const XP_UCHAR* name,
+addPlayer( XW_DUtilCtxt* XP_UNUSED_DBG(dutil), KPState* state,
-           const CommsAddrRec* addr, XP_U32 newestMod )
+           const XP_UCHAR* name, const CommsAddrRec* addr, XP_U32 newestMod )
 {
     XP_LOGFF( "(name=%s, newestMod: %d)", name, newestMod );
-    KnownPlayer* withSameDevID = NULL;
+    AddData ad = {.name = name, .addr = addr, };
-    KnownPlayer* withSameName = NULL;
+    dll_map( &state->players->links, addProc, NULL, &ad );
 
-    for ( KnownPlayer* kp = state->players;
+    XP_LOGFF( "withSameDevID: %p; withSameName: %p",
-          !!kp && (!withSameDevID || !withSameName);
+              ad.withSameDevID, ad.withSameName );
-          kp = (KnownPlayer*)kp->links.next ) {
-        if ( 0 == XP_STRCMP( kp->name, name ) ) {
-            withSameName = kp;
-        }
-        if ( addr->u.mqtt.devID == kp->addr.u.mqtt.devID ) {
-            withSameDevID = kp;
-        }
-    }
-
-    XP_LOGFF( "withSameDevID: %p; withSameName: %p", withSameDevID, withSameName );
 
     XP_UCHAR tmpName[64];
-    if ( !!withSameDevID ) {    /* only one allowed */
+    if ( !!ad.withSameDevID ) {    /* only one allowed */
-        XP_Bool isNewer = newestMod > withSameDevID->newestMod;
+        XP_Bool isNewer = newestMod > ad.withSameDevID->newestMod;
-        XP_Bool changed = augmentAddr( &withSameDevID->addr, addr, isNewer );
+        XP_Bool changed = augmentAddr( &ad.withSameDevID->addr,
+                                       addr, isNewer );
         if ( isNewer ) {
-            XP_LOGFF( "updating newestMod from %d to %d", withSameDevID->newestMod, newestMod );
+            XP_LOGFF( "updating newestMod from %d to %d",
-            withSameDevID->newestMod = newestMod;
+                      ad.withSameDevID->newestMod, newestMod );
+            ad.withSameDevID->newestMod = newestMod;
             changed = XP_TRUE;
         } else {
             XP_LOGFF( "not newer" );
         }
         state->dirty = changed || state->dirty;
     } else {
-        if ( !!withSameName ) {
+        if ( !!ad.withSameName ) {
         /* Same name but different devID? Create a unique name */
             makeUniqueName( state, name, tmpName, VSIZE(tmpName) );
             name = tmpName;
@@ -298,12 +314,13 @@ typedef struct _GetState {
 } GetState;
 
 
-static void
+static ForEachAct
 getProc( const DLHead* dl, void* closure )
 {
     GetState* gsp = (GetState*)closure;
     const KnownPlayer* kp = (KnownPlayer*)dl;
     gsp->players[gsp->indx++] = kp->name;
+    return FEA_OK;
 }
 
 static void
@@ -311,10 +328,9 @@ getPlayersImpl( const KPState* state, const XP_UCHAR** players,
                 XP_U16* nFound )
 {
     if ( state->nPlayers <= *nFound && !!players ) {
-        GetState gs = { .players = players,
+        GetState gs = { .players = players, .indx = 0, };
-            .indx = 0,
+        DLHead* head = dll_map( &state->players->links, getProc, NULL, &gs );
-        };
+        XP_ASSERT( head == &state->players->links );
-        dll_map( &state->players->links, getProc, &gs );
     }
     *nFound = state->nPlayers;
 }
@@ -336,17 +352,31 @@ kplr_getNames( XW_DUtilCtxt* dutil, XWEnv xwe, XP_Bool byDate,
     releaseState( dutil, xwe, state );
 }
 
+typedef struct _FindState {
+    const XP_UCHAR* name;
+    const KnownPlayer* result;
+} FindState;
+
+static ForEachAct
+findProc( const DLHead* dl, void* closure )
+{
+    ForEachAct result = FEA_OK;
+    FindState* fsp = (FindState*)closure;
+    const KnownPlayer* kp = (KnownPlayer*)dl;
+    if ( 0 == XP_STRCMP( kp->name, fsp->name ) ) {
+        fsp->result = kp;
+        result = FEA_EXIT;
+    }
+    return result;
+}
+
 static KnownPlayer*
 findByName( KPState* state, const XP_UCHAR* name )
 {
-    KnownPlayer* result = NULL;
+    FindState fs = { .name = name, };
-    for ( KnownPlayer* kp = state->players; !!kp && !result;
+    DLHead* head = dll_map( &state->players->links, findProc, NULL, &fs );
-          kp = (KnownPlayer*)kp->links.next ) {
+    XP_ASSERT( head == &state->players->links );
-        if ( 0 == XP_STRCMP( kp->name, name ) ) {
+    return (KnownPlayer*)fs.result;
-            result = kp;
-        }
-    }
-    return result;
 }
 
 XP_Bool
@@ -368,26 +398,40 @@ kplr_getAddr( XW_DUtilCtxt* dutil, XWEnv xwe, const XP_UCHAR* name,
     return found;
 }
 
-const XP_UCHAR*
+typedef struct _MDevState {
-kplr_nameForMqttDev( XW_DUtilCtxt* dutil, XWEnv xwe, const XP_UCHAR* mqttDevID )
-{
-    const XP_UCHAR* name = NULL;
     MQTTDevID devID;
-    if ( strToMQTTCDevID( mqttDevID, &devID ) ) {
+    const XP_UCHAR* name;
-        KPState* state = loadState( dutil, xwe );
+} MDevState;
-        for ( KnownPlayer* kp = state->players; !!kp && !name;
+
-              kp = (KnownPlayer*)kp->links.next ) {
+static ForEachAct
+mqttProc( const DLHead* dl, void* closure )
+{
+    ForEachAct result = FEA_OK;
+    const KnownPlayer* kp = (KnownPlayer*)dl;
+    MDevState* msp = (MDevState*)closure;
     const CommsAddrRec* addr = &kp->addr;
-            if ( addr_hasType( addr, COMMS_CONN_MQTT ) ) {
+    if ( addr_hasType( addr, COMMS_CONN_MQTT )
-                if ( 0 == XP_MEMCMP( &addr->u.mqtt.devID, &devID, sizeof(devID) ) ) {
+         && 0 == XP_MEMCMP( &addr->u.mqtt.devID, &msp->devID,
-                    name = kp->name;
+                            sizeof(msp->devID) ) ) {
-                }
+        msp->name = kp->name;
-            }
+        result = FEA_EXIT;
     }
+    return result;
+}
+
+const XP_UCHAR*
+kplr_nameForMqttDev( XW_DUtilCtxt* dutil, XWEnv xwe,
+                     const XP_UCHAR* mqttDevID )
+{
+    MDevState ms = {0};
+    if ( strToMQTTCDevID( mqttDevID, &ms.devID ) ) {
+        KPState* state = loadState( dutil, xwe );
+        DLHead* head = dll_map( &state->players->links, mqttProc, NULL, &ms );
+        XP_ASSERT( head == &state->players->links );
         releaseState( dutil, xwe, state );
     }
-    LOG_RETURNF( "%s", name );
+    LOG_RETURNF( "%s", ms.name );
-    return name;
+    return ms.name;
 }
 
 static void
@@ -420,31 +464,45 @@ kplr_renamePlayer( XW_DUtilCtxt* dutil, XWEnv xwe, const XP_UCHAR* oldName,
     return result;
 }
 
+typedef struct _DelState {
+    XW_DUtilCtxt* dutil;
+    const XP_UCHAR* name;
+    KPState* state;
+} DelState;
+
+static ForEachAct
+delMapProc( const DLHead* dl, void* closure )
+{
+    ForEachAct result = FEA_OK;
+
+    const KnownPlayer* kp = (KnownPlayer*)dl;
+    DelState* dsp = (DelState*)closure;
+    if ( 0 == XP_STRCMP( kp->name, dsp->name ) ) {
+        result = FEA_REMOVE | FEA_EXIT;
+        --dsp->state->nPlayers;
+        dsp->state->dirty = XP_TRUE;
+    }
+    return result;
+}
+
+static void
+delFreeProc( DLHead* elem, void* closure )
+{
+    DelState* dsp = (DelState*)closure;
+    freeKP( dsp->dutil, (KnownPlayer*)elem );
+}
+
 KP_Rslt
 kplr_deletePlayer( XW_DUtilCtxt* dutil, XWEnv xwe, const XP_UCHAR* name )
 {
-    KP_Rslt result = KP_NAME_NOT_FOUND;
+    KP_Rslt result = KP_OK;
-    KnownPlayer* doomed = NULL;
     KPState* state = loadState( dutil, xwe );
 
-    for ( KnownPlayer* kp = state->players; !!kp;
+    DelState ds = { .name = name, .state = state, .dutil = dutil,};
-          kp = (KnownPlayer*)kp->links.next ) {
-        if ( 0 == XP_STRCMP( kp->name, name ) ) {
-            doomed = kp;
     state->players = (KnownPlayer*)
-                dll_remove( &state->players->links, &doomed->links );
+        dll_map( &state->players->links, delMapProc, delFreeProc, &ds );
-            --state->nPlayers;
-            state->dirty = XP_TRUE;
-            result = KP_OK;
-            break;
-        }
-    }
     releaseState( dutil, xwe, state );
 
-    XP_ASSERT( !!doomed );
-    if ( !!doomed ) {
-        freeKP( dutil, doomed );
-    }
     return result;
 }
 

xwords4/linux/linuxmain.c

@@ -2598,10 +2598,11 @@ compByNameRev( const DLHead* dl1, const DLHead* dl2 )
     return strcmp( ((TestThing*)dl2)->name, ((TestThing*)dl1)->name );
 }
 
-static void
+static ForEachAct
 mapProc( const DLHead* dl, void* XP_UNUSED(closure))
 {
     XP_LOGFF( "name: %s", ((TestThing*)dl)->name );
+    return FEA_OK;
 }
 
 static TestThing*
@@ -2609,7 +2610,7 @@ removeAndMap( TestThing* list, TestThing* node )
 {
     XP_LOGFF( "removing %s", node->name );
     list = (TestThing*)dll_remove( &list->links, &node->links );
-    dll_map( &list->links, mapProc, NULL );
+    dll_map( &list->links, mapProc, NULL, NULL );
     return list;
 }
 
@@ -2627,10 +2628,10 @@ testDLL()
     for ( int ii = 0; ii < VSIZE(tss); ++ii ) {
         list = (TestThing*)dll_insert( &list->links, &tss[ii].links, compByName );
     }
-    dll_map( &list->links, mapProc, NULL );
+    dll_map( &list->links, mapProc, NULL, NULL );
 
     list = (TestThing*)dll_sort( &list->links, compByNameRev );
-    dll_map( &list->links, mapProc, NULL );
+    dll_map( &list->links, mapProc, NULL, NULL );
 
     list = removeAndMap( list, &tss[0] );
     list = removeAndMap( list, &tss[2] );