Re: PATCH: fix stale pointer access in fp_TextRun.cpp

The fp_TextRun::m_pLanguage is supposed to be pointer directly into the
static tables of UT_Language class (as is documentand in fp_TextRun); as
such the pointer has a guaranteed life span same as the application.

For some reason Martin has changed that on Sep 14 as part of his 'Only
grammar check English' commit, adding the strdup calls. There really is
no reason why we should be duplicating the language code in fp_TextRun,
and apart from the unnecessary memory usage, storing pointers directly
into UT_Lang means we are able to simply compare pointers instead of
calling UT_strcmp on them in fp_TextRun::canMergeWithNext(); we do an
awful lot of these comparisons and it makes sense to me.

I would really like to revert both Row's and Martin's changes.

Tomas

Robert Wilhelm wrote:
>
> Attached patch fixes the valgrind trace from
> http://bugzilla.abisource.com/show_bug.cgi?id=9434#c9
>
> Ok to commit?
>
> row
>
> RCS file: /cvsroot/abi/src/text/fmt/xp/fp_TextRun.cpp,v
> retrieving revision 1.360
> diff -u -r1.360 fp_TextRun.cpp
> --- fp_TextRun.cpp 15 Sep 2005 02:37:51 -0000 1.360
> +++ fp_TextRun.cpp 28 Sep 2005 17:50:07 -0000
> @@ -251,10 +251,9 @@
> const XML_Char * pszLanguage =
> PP_evalProperty("lang",pSpanAP,pBlockAP,p SectionAP, pDoc, true);
>
> const XML_Char * pszOldLanguage = m_pLanguage;
> - FREEP(m_pLanguage);
> - m_pLanguage = UT_strdup(lls.getCodeFromCode(pszLanguage));
> - xxx_UT_DEBUGMSG(("!!!!!!!! Language of run set to %s pointer %x
> run %x \ n",getLanguage(),m_pLanguage,this));
> - if(pszOldLanguage && (UT_strcmp(m_pLanguage,pszOldLanguage) !=
> 0))
> + const XML_Char * pszNewLanguage =
> UT_strdup(lls.getCodeFromCode(pszLangu age));
> + xxx_UT_DEBUGMSG(("!!!!!!!! Language of run set to %s pointer %x
> run %x \ n",getLanguage(),pszNewLanguage,this));
> + if(pszOldLanguage && (UT_strcmp(pszNewLanguage,pszOldLanguage) !
> = 0))
> {
> UT_uint32 reason = 0;
> if( getBlock()->getDocLayout()->getAutoSpellCheck())
> @@ -268,6 +267,8 @@
>
> getBlock()->getDocLayout()->queueBlockForBackgroundCheck(reason,
> getBlock());
> bChanged = true;
> }
> + FREEP(m_pLanguage);
> + m_pLanguage = pszNewLanguage;
>
>
>

        
        
                
___________________________________________________________
Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com
Received on Thu Sep 29 08:38:16 2005