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

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