[Libreoffice] [PATCH] Replace (Byte)String with O(U)String

Stephan Bergmann sbergman at redhat.com
Tue Jan 17 04:46:27 PST 2012


On 01/17/2012 11:06 AM, Chr. Rossmanith wrote:
> an excursion from vcl to unotools was necessary. Could someone please
> review this little patch? xub_StrLen is replaced as well with sal_Int32.

Two things about the patch:

> -void ConvertChar::RecodeString( String& rStr, xub_StrLen nIndex, xub_StrLen nLen ) const
> +void ConvertChar::RecodeString( rtl::OUString& rStr, sal_Int32 nIndex, sal_Int32 nLen ) const
>  {
> -    sal_uLong nLastIndex = (sal_uLong)nIndex + nLen;
> -    if( nLastIndex > rStr.Len() )
> -        nLastIndex = rStr.Len();
> +    sal_Int32 nLastIndex = nIndex + nLen;
> +    if( nLastIndex > rStr.getLength() )
> +        nLastIndex = rStr.getLength();

The original nIndex, nLen were 16 bit, so their sum was guaranteed to 
fit into 32 bit sal_uLong.  There is no such guarantee with the new 
nIndex, nLen of 32 bit.  But looking around, the only call of 
RecodeString is

   mpFontEntry->mpConversion->RecodeString( aStr, 0, aStr.Len() );

in vcl/source/gdi/outdev3.cxx.  So this can all go away, changing the 
signature of RecodeString to

   rtl::OUString RecodeString(rtl::OUString const &)

and adapting the single call side accordingly (also getting rid of the 
in-out parameter).

>      for(; nIndex < nLastIndex; ++nIndex )
>      {
> -        sal_Unicode cOrig = rStr.GetChar( nIndex );
> +        sal_Unicode cOrig = rStr[nIndex];
>          // only recode symbols and their U+00xx aliases
>          if( ((cOrig < 0x0020) || (cOrig > 0x00FF))
>          &&  ((cOrig < 0xF020) || (cOrig > 0xF0FF)) )
> @@ -1390,8 +1391,12 @@ void ConvertChar::RecodeString( String& rStr, xub_StrLen nIndex, xub_StrLen nLen
>
>          // recode a symbol
>          sal_Unicode cNew = RecodeChar( cOrig );
> -        if( cOrig != cNew )
> -            rStr.SetChar( nIndex, cNew );
> +        if( cOrig != cNew ) {
> +            //            rStr[nIndex] = cNew;
> +            rtl::OUStringBuffer aTmpStr(rStr.getStr());
> +            aTmpStr[nIndex] = cNew;
> +            rStr = aTmpStr.makeStringAndClear();
> +        }

Constructing a temporary OUStringBuffer for each character that needs 
conversion smells somewhat inefficient.  An alternative is to put the 
original rStr into an OUStringBuffer once and iterate over that one.

Stephan


More information about the LibreOffice mailing list