From 6d98631640fc74617652b874b2d2fe46806cc303 Mon Sep 17 00:00:00 2001 From: Eric House Date: Thu, 10 Nov 2011 18:24:45 -0800 Subject: [PATCH 1/4] sanity check: make sure tiles are within range. --- xwords4/common/dictnry.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/xwords4/common/dictnry.c b/xwords4/common/dictnry.c index 97d769f29..af2bf67a7 100644 --- a/xwords4/common/dictnry.c +++ b/xwords4/common/dictnry.c @@ -705,13 +705,15 @@ checkSanity( DictionaryCtxt* dict, const XP_U32 numEdges ) { XP_U32 ii; XP_Bool passed = XP_TRUE; + XP_U16 nFaces = dict_numTileFaces( dict ); + array_edge* edge = dict->base; Tile prevTile = 0; for ( ii = 0; ii < numEdges && passed; ++ii ) { Tile tile = EDGETILE( dict, edge ); - if ( tile < prevTile ) { - XP_LOGF( "%s: node %ld of %ld has out-of-order tile", __func__, - ii, numEdges ); + if ( tile < prevTile || tile >= nFaces ) { + XP_LOGF( "%s: node %ld (out of %ld) has too-large or " + "out-of-order tile", __func__, ii, numEdges ); passed = XP_FALSE; break; } @@ -719,8 +721,8 @@ checkSanity( DictionaryCtxt* dict, const XP_U32 numEdges ) unsigned long index = dict_index_from( dict, edge ); if ( index >= numEdges ) { - XP_LOGF( "%s: node %ld of %ld has too-high index", __func__, - ii, numEdges ); + XP_LOGF( "%s: node %ld (out of %ld) has too-high index %ld", __func__, + ii, numEdges, index ); passed = XP_FALSE; break; } From cee738942eb02f322bc5fda8695d303c47422a04 Mon Sep 17 00:00:00 2001 From: Eric House Date: Thu, 10 Nov 2011 18:26:32 -0800 Subject: [PATCH 2/4] back out earlier change using redo: it was broken. Add assert to catch problem with passing ptr to tiles as src of tiles. --- xwords4/common/model.c | 81 ++++++++++++++++++++++++++---------------- xwords4/common/model.h | 4 +++ 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/xwords4/common/model.c b/xwords4/common/model.c index 12a3506db..c40976c8c 100644 --- a/xwords4/common/model.c +++ b/xwords4/common/model.c @@ -1495,18 +1495,19 @@ static void makeTileTrade( ModelCtxt* model, XP_S16 player, const TrayTileSet* oldTiles, const TrayTileSet* newTiles ) { - XP_U16 i; + XP_U16 ii; XP_U16 nTiles; XP_ASSERT( newTiles->nTiles == oldTiles->nTiles ); + XP_ASSERT( oldTiles != &model->players[player].trayTiles ); - for ( nTiles = newTiles->nTiles, i = 0; i < nTiles; ++i ) { - Tile oldTile = oldTiles->tiles[i]; + for ( nTiles = newTiles->nTiles, ii = 0; ii < nTiles; ++ii ) { + Tile oldTile = oldTiles->tiles[ii]; XP_S16 tileIndex = model_trayContains( model, player, oldTile ); XP_ASSERT( tileIndex >= 0 ); model_removePlayerTile( model, player, tileIndex ); - model_addPlayerTile( model, player, tileIndex, newTiles->tiles[i] ); + model_addPlayerTile( model, player, tileIndex, newTiles->tiles[ii] ); } } /* makeTileTrade */ @@ -1545,6 +1546,21 @@ model_getPlayerTiles( const ModelCtxt* model, XP_S16 turn ) return (const TrayTileSet*)&player->trayTiles; } /* model_getPlayerTile */ +#ifdef DEBUG +XP_UCHAR* +formatTileSet( const TrayTileSet* tiles, XP_UCHAR* buf, XP_U16 len ) +{ + XP_U16 ii, used; + for ( ii = 0, used = 0; ii < tiles->nTiles && used < len; ++ii ) { + used += XP_SNPRINTF( &buf[used], len - used, "%d,", tiles->tiles[ii] ); + } + if ( used > len ) { + buf[len-1] = '\0'; + } + return buf; +} +#endif + static void addPlayerTile( ModelCtxt* model, XP_S16 turn, XP_S16 index, const Tile tile ) { @@ -1892,6 +1908,19 @@ printMovePost( ModelCtxt* model, XP_U16 XP_UNUSED(moveN), printString( stream, (XP_UCHAR*)XP_CR ); } /* printMovePost */ +static void +copyStack( ModelCtxt* model, StackCtxt* destStack, const StackCtxt* srcStack ) +{ + XWStreamCtxt* stream = mem_stream_make( MPPARM(model->vol.mpool) + util_getVTManager(model->vol.util), + NULL, 0, NULL ); + + stack_writeToStream( (StackCtxt*)srcStack, stream ); + stack_loadFromStream( destStack, stream ); + + stream_destroy( stream ); +} /* copyStack */ + static ModelCtxt* makeTmpModel( ModelCtxt* model, XWStreamCtxt* stream, MovePrintFuncPre mpf_pre, MovePrintFuncPost mpf_post, @@ -1953,21 +1982,6 @@ getFirstWord( const XP_UCHAR* word, XP_Bool isLegal, return XP_TRUE; } -static void -redoEntries( ModelCtxt* model, StackCtxt* stack, XP_U16 nBefore, - XP_U16 nAfter, WordNotifierInfo* ni ) -{ - while ( nAfter < nBefore ) { - StackEntry entry; - if ( ! stack_redo( stack, &entry ) ) { - XP_ASSERT( 0 ); - break; - } - modelAddEntry( model, nAfter++, &entry, XP_FALSE, NULL, ni, - NULL, NULL, NULL ); - } -} - static void scoreLastMove( ModelCtxt* model, MoveInfo* moveInfo, XP_U16 howMany, XP_UCHAR* buf, XP_U16* bufLen ) @@ -1983,29 +1997,28 @@ scoreLastMove( ModelCtxt* model, MoveInfo* moveInfo, XP_U16 howMany, const XP_UCHAR* format; WordNotifierInfo notifyInfo; FirstWordData data; - StackCtxt* stack = model->vol.stack; - XP_U16 nEntriesBefore = stack_getNEntries( stack ); - XP_U16 nEntriesAfter; + + ModelCtxt* tmpModel = makeTmpModel( model, NULL, NULL, NULL, NULL ); XP_U16 turn; XP_S16 moveNum = -1; - - if ( !model_undoLatestMoves( model, NULL, howMany, - &turn, &moveNum ) ) { + copyStack( model, tmpModel->vol.stack, model->vol.stack ); + + if ( !model_undoLatestMoves( tmpModel, NULL, howMany, &turn, + &moveNum ) ) { XP_ASSERT( 0 ); } - nEntriesAfter = stack_getNEntries( stack ); data.word[0] = '\0'; notifyInfo.proc = getFirstWord; notifyInfo.closure = &data; - score = figureMoveScore( model, turn, moveInfo, (EngineCtxt*)NULL, + score = figureMoveScore( tmpModel, turn, moveInfo, (EngineCtxt*)NULL, (XWStreamCtxt*)NULL, ¬ifyInfo ); + model_destroy( tmpModel ); + format = util_getUserString( model->vol.util, STRSD_SUMMARYSCORED ); *bufLen = XP_SNPRINTF( buf, *bufLen, format, data.word, score ); - - redoEntries( model, stack, nEntriesBefore, nEntriesAfter, NULL ); } } /* scoreLastMove */ @@ -2134,7 +2147,15 @@ model_listWordsThrough( ModelCtxt* model, XP_U16 col, XP_U16 row, /* Now push the undone moves back into the model one at a time. recordWord() will add each played word to the stream as it's scored */ - redoEntries( model, stack, nEntriesBefore, nEntriesAfter, &ni ); + while ( nEntriesAfter < nEntriesBefore ) { + StackEntry entry; + if ( ! stack_redo( stack, &entry ) ) { + XP_ASSERT( 0 ); + break; + } + modelAddEntry( model, nEntriesAfter++, &entry, XP_FALSE, NULL, &ni, + NULL, NULL, NULL ); + } } } /* model_listWordsThrough */ #endif diff --git a/xwords4/common/model.h b/xwords4/common/model.h index caf945174..614829a0a 100644 --- a/xwords4/common/model.h +++ b/xwords4/common/model.h @@ -145,6 +145,10 @@ void model_moveTileOnTray( ModelCtxt* model, XP_S16 turn, XP_S16 indexCur, player. Don't even think about modifying the array!!!! */ const TrayTileSet* model_getPlayerTiles( const ModelCtxt* model, XP_S16 turn ); +#ifdef DEBUG +XP_UCHAR* formatTileSet( const TrayTileSet* tiles, XP_UCHAR* buf, XP_U16 len ); +#endif + void model_sortTiles( ModelCtxt* model, XP_S16 turn ); XP_U16 model_getNumTilesInTray( ModelCtxt* model, XP_S16 turn ); XP_U16 model_getNumTilesTotal( ModelCtxt* model, XP_S16 turn ); From 619c25ee494cbdb9d1cf3a7e79c8cbc3b23424fe Mon Sep 17 00:00:00 2001 From: Eric House Date: Thu, 10 Nov 2011 18:28:11 -0800 Subject: [PATCH 3/4] add a bit of logging to help deciphering test app problems. --- xwords4/common/server.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xwords4/common/server.c b/xwords4/common/server.c index daf7a65ad..2e911ec63 100644 --- a/xwords4/common/server.c +++ b/xwords4/common/server.c @@ -728,6 +728,7 @@ makeRobotMove( ServerCtxt* server ) /* trade if unable to find a move */ if ( trade ) { TrayTileSet oldTiles = *model_getPlayerTiles( model, turn ); + XP_LOGF( "%s: robot trading %d tiles", __func__, oldTiles.nTiles ); result = server_commitTrade( server, &oldTiles ); /* Quick hack to fix gremlin bug where all-robot game seen none @@ -749,6 +750,7 @@ makeRobotMove( ServerCtxt* server ) if ( canMove || NPASSES_OK(server) ) { model_makeTurnFromMoveInfo( model, turn, &newMove ); + XP_LOGF( "%s: robot making %d tile move", __func__, newMove.nTiles ); if ( !!stream ) { XWStreamCtxt* wordsStream = mkServerStream( server ); @@ -1829,6 +1831,7 @@ readMoveInfo( ServerCtxt* server, XWStreamCtxt* stream, if ( isTrade ) { traySetFromStream( stream, tradedTiles ); + XP_LOGF( "%s: got trade of %d tiles", __func__, tradedTiles->nTiles ); } else { legalMove = stream_getBits( stream, 1 ); model_makeTurnFromStream( server->vol.model, whoMoved, stream ); From 5be5e9fdeba9fbcf9cc0614114d9c73da4607bf2 Mon Sep 17 00:00:00 2001 From: Eric House Date: Thu, 10 Nov 2011 18:39:35 -0800 Subject: [PATCH 4/4] cleanup script output --- xwords4/linux/scripts/discon_ok2.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xwords4/linux/scripts/discon_ok2.sh b/xwords4/linux/scripts/discon_ok2.sh index 0131ac931..2309cd79a 100755 --- a/xwords4/linux/scripts/discon_ok2.sh +++ b/xwords4/linux/scripts/discon_ok2.sh @@ -279,14 +279,14 @@ check_game() { fi if [ -n "$OTHERS" ]; then - echo -n "Closing $CONNNAME: " + echo -n "Closing $CONNNAME [$(date)]: " # kill_from_logs $OTHERS $KEY for ID in $OTHERS $KEY; do echo -n "${LOGS[$ID]}, " kill_from_log ${LOGS[$ID]} || true close_device $ID $DONEDIR "game over" done - date + echo "" # XWRELAY_ERROR_DELETED may be old elif grep -q 'relay_error_curses(XWRELAY_ERROR_DELETED)' $LOG; then echo "deleting $LOG $(connName $LOG) b/c another resigned"