From ea0b176b2a103b681c165b2519ae88847cc8363d Mon Sep 17 00:00:00 2001 From: Eric House Date: Fri, 28 Jun 2019 21:22:45 -0700 Subject: [PATCH 1/2] debug-build-only macros to find proto-based crashes When a stream reader and writer are out of sync it often shows up as trying to read off the end of the stream (firing an assert.) It's useful to know where that's coming from on android where there's usually no stack trace in the jin world. So pass __func__ in using macros, and log before asserting. Crude but quick and already useful. --- xwords4/common/memstream.c | 26 +++++++++++++++++++------- xwords4/common/xwstream.h | 29 +++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/xwords4/common/memstream.c b/xwords4/common/memstream.c index f7cd5f53b..399e0978c 100644 --- a/xwords4/common/memstream.c +++ b/xwords4/common/memstream.c @@ -113,7 +113,7 @@ mem_stream_make_sized( MPFORMAL VTableMgr* vtmgr, XP_U16 startSize, } static void -mem_stream_getBytes( XWStreamCtxt* p_sctx, void* where, XP_U16 count ) +mem_stream_getBytes( DBG_PROC_FORMAL XWStreamCtxt* p_sctx, void* where, XP_U16 count ) { MemStreamCtxt* stream = (MemStreamCtxt*)p_sctx; @@ -121,8 +121,20 @@ mem_stream_getBytes( XWStreamCtxt* p_sctx, void* where, XP_U16 count ) stream->nReadBits = 0; } +#ifdef MEM_DEBUG + if( stream->curReadPos + count > stream->nBytesAllocated ) { + XP_LOGF( "%s(): caller: %s()", __func__, DBG_PROC_VAL_NOCOMMA ); + XP_ASSERT(0); + } + if ( stream->curReadPos + count > stream->nBytesWritten ) { + XP_LOGF( "%s(): caller: %s()", __func__, DBG_PROC_VAL_NOCOMMA ); + XP_ASSERT(0); + } +#else XP_ASSERT( stream->curReadPos + count <= stream->nBytesAllocated ); XP_ASSERT( stream->curReadPos + count <= stream->nBytesWritten ); +#endif + XP_MEMCPY( where, stream->buf + stream->curReadPos, count ); stream->curReadPos += count; @@ -130,27 +142,27 @@ mem_stream_getBytes( XWStreamCtxt* p_sctx, void* where, XP_U16 count ) } /* mem_stream_getBytes */ static XP_U8 -mem_stream_getU8( XWStreamCtxt* p_sctx ) +mem_stream_getU8( DBG_PROC_FORMAL XWStreamCtxt* p_sctx ) { XP_U8 result; - mem_stream_getBytes( p_sctx, &result, sizeof(result) ); + mem_stream_getBytes( DBG_PROC_VAL p_sctx, &result, sizeof(result) ); return result; } /* mem_stream_getU8 */ static XP_U16 -mem_stream_getU16( XWStreamCtxt* p_sctx ) +mem_stream_getU16( DBG_PROC_FORMAL XWStreamCtxt* p_sctx ) { XP_U16 result; - mem_stream_getBytes( p_sctx, &result, sizeof(result) ); + mem_stream_getBytes( DBG_PROC_VAL p_sctx, &result, sizeof(result) ); return XP_NTOHS(result); } /* mem_stream_getU16 */ static XP_U32 -mem_stream_getU32( XWStreamCtxt* p_sctx ) +mem_stream_getU32( DBG_PROC_FORMAL XWStreamCtxt* p_sctx ) { XP_U32 result; - mem_stream_getBytes( p_sctx, &result, sizeof(result) ); + mem_stream_getBytes( DBG_PROC_VAL p_sctx, &result, sizeof(result) ); return XP_NTOHL( result ); } /* mem_stream_getU32 */ diff --git a/xwords4/common/xwstream.h b/xwords4/common/xwstream.h index a9177e644..3c6098ee4 100644 --- a/xwords4/common/xwstream.h +++ b/xwords4/common/xwstream.h @@ -40,14 +40,27 @@ typedef XP_U8 PosWhich; # define DBG_LINE_FILE_PARM #endif + +#ifdef MEM_DEBUG +# define DBG_PROC __func__, +# define DBG_PROC_FORMAL const char* proc, +# define DBG_PROC_VAL_NOCOMMA proc +# define DBG_PROC_VAL DBG_PROC_VAL_NOCOMMA, +#else +# define DBG_PROC +# define DBG_PROC_FORMAL +foo +#endif + + typedef struct StreamCtxVTable { void (*m_stream_destroy)( XWStreamCtxt* dctx ); - XP_U8 (*m_stream_getU8)( XWStreamCtxt* dctx ); - void (*m_stream_getBytes)( XWStreamCtxt* dctx, void* where, + XP_U8 (*m_stream_getU8)( DBG_PROC_FORMAL XWStreamCtxt* dctx ); + void (*m_stream_getBytes)( DBG_PROC_FORMAL XWStreamCtxt* dctx, void* where, XP_U16 count ); - XP_U16 (*m_stream_getU16)( XWStreamCtxt* dctx ); - XP_U32 (*m_stream_getU32)( XWStreamCtxt* dctx ); + XP_U16 (*m_stream_getU16)( DBG_PROC_FORMAL XWStreamCtxt* dctx ); + XP_U32 (*m_stream_getU32)( DBG_PROC_FORMAL XWStreamCtxt* dctx ); XP_U32 (*m_stream_getBits)( XWStreamCtxt* dctx, XP_U16 nBits ); #if defined DEBUG void (*m_stream_copyBits)( const XWStreamCtxt* dctx, XWStreamPos endPos, @@ -102,16 +115,16 @@ struct XWStreamCtxt { (sc)->vtable->m_stream_destroy(sc) #define stream_getU8(sc) \ - (sc)->vtable->m_stream_getU8(sc) + (sc)->vtable->m_stream_getU8(DBG_PROC (sc)) #define stream_getBytes(sc, wh, c ) \ - (sc)->vtable->m_stream_getBytes((sc), (wh), (c)) + (sc)->vtable->m_stream_getBytes(DBG_PROC (sc), (wh), (c)) #define stream_getU16(sc) \ - (sc)->vtable->m_stream_getU16(sc) + (sc)->vtable->m_stream_getU16(DBG_PROC sc) #define stream_getU32(sc) \ - (sc)->vtable->m_stream_getU32(sc) + (sc)->vtable->m_stream_getU32(DBG_PROC sc) #define stream_getBits(sc, n) \ (sc)->vtable->m_stream_getBits((sc), (n)) From d240e56527b41c3edd50eb374554f4c0c22d13d6 Mon Sep 17 00:00:00 2001 From: Eric House Date: Sat, 29 Jun 2019 12:30:08 -0700 Subject: [PATCH 2/2] fix NPE with empty wordlist (and add note for Greek) --- .../android/xw4/DictBrowseDelegate.java | 22 +++++++++++-------- xwords4/dawg/Greek/Makefile | 2 ++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DictBrowseDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DictBrowseDelegate.java index 29ff4909b..be0c70e82 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DictBrowseDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DictBrowseDelegate.java @@ -90,14 +90,6 @@ public class DictBrowseDelegate extends DelegateBase setTitle( getString( format, m_name, m_nWords, m_browseState.m_minShown, m_browseState.m_maxShown )); - - String desc = XwJNI.dict_iter_getDesc( m_dictClosure ); - if ( null != desc ) { - TextView view = (TextView)findViewById( R.id.desc ); - Assert.assertNotNull( view ); - view.setVisibility( View.VISIBLE ); - view.setText( desc ); - } } public Object getItem( int position ) @@ -177,6 +169,15 @@ public class DictBrowseDelegate extends DelegateBase m_dictClosure = XwJNI.dict_iter_init( pairs.m_bytes[0], name, pairs.m_paths[0] ); + String desc = XwJNI.dict_iter_getDesc( m_dictClosure ); + Log.d( TAG, "got desc: %s", desc ); + if ( null != desc ) { + TextView view = (TextView)findViewById( R.id.desc ); + Assert.assertNotNull( view ); + view.setVisibility( View.VISIBLE ); + view.setText( desc ); + } + m_browseState = DBUtils.dictsGetOffset( m_activity, name, m_loc ); boolean newState = null == m_browseState; if ( newState ) { @@ -220,7 +221,8 @@ public class DictBrowseDelegate extends DelegateBase protected void onPause() { - if ( null != m_browseState ) { // already saved? + if ( null != m_browseState // already saved? + && null != m_list ) { // there are words? (don't NPE on empty dict) m_browseState.m_pos = m_list.getFirstVisiblePosition(); View view = m_list.getChildAt( 0 ); m_browseState.m_top = (view == null) ? 0 : view.getTop(); @@ -274,6 +276,7 @@ public class DictBrowseDelegate extends DelegateBase ////////////////////////////////////////////////// // AdapterView.OnItemSelectedListener interface ////////////////////////////////////////////////// + @Override public void onItemSelected( AdapterView parent, View view, int position, long id ) { @@ -296,6 +299,7 @@ public class DictBrowseDelegate extends DelegateBase } } + @Override public void onNothingSelected( AdapterView parent ) { } diff --git a/xwords4/dawg/Greek/Makefile b/xwords4/dawg/Greek/Makefile index 3d92bfa8d..ef74e748d 100644 --- a/xwords4/dawg/Greek/Makefile +++ b/xwords4/dawg/Greek/Makefile @@ -20,6 +20,8 @@ XWLANG = Greek LANGCODE = el_GK ENC = UTF-8 +DICTNOTE = "Tiles for Greek (no words, so robot and hint won't work)" + # DICT2DAWGARGS = -lang $(LANGCODE) # DICT2DAWGARGS = -debug