Re: [Libreoffice] [PATCH] Replace ByteString with rtl::OString

Hi Chr.,

On Monday, 2012-01-02 22:05:15 +0100, Chr. Rossmanith wrote:

> could someone please review this small patch and give feed back.

Sure.

> diff --git a/vcl/generic/print/genprnpsp.cxx b/vcl/generic/print/genprnpsp.cxx
> [...]
> -static String getPdfDir( const PrinterInfo& rInfo )
> +static rtl::OUString getPdfDir( const PrinterInfo& rInfo )
>  {
> -    String aDir;
> +    rtl::OUString aDir;
>      sal_Int32 nIndex = 0;
>      while( nIndex != -1 )
>      {
> -        OUString aToken( rInfo.m_aFeatures.getToken( 0, ',', nIndex ) );
> +        rtl::OUString aToken( rInfo.m_aFeatures.getToken( 0, ',', nIndex ) );

Prefixing with rtl:: in those places shouldn't be necessary, as further
above the file has a   using ::rtl::OUString;   directive.

>          if( ! aToken.compareToAscii( "pdf=", 4 ) )
>          {
>              sal_Int32 nPos = 0;
>              aDir = aToken.getToken( 1, '=', nPos );
> -            if( ! aDir.Len() )
> -                aDir = String( ByteString( getenv( "HOME" ) ), osl_getThreadTextEncoding() );
> +            if( aDir.isEmpty() )
> +                aDir = rtl::OUString( getenv( "HOME" ), 4, osl_getThreadTextEncoding() );

Trapped ;-)  The string isn't constructed of the word HOME, but the
content of the HOME environment variable instead, so assuming 4 here
isn't correct.

Besides that, already the original code didn't deal with a possible NULL
pointer returned by getenv() in case a HOME variable wasn't set at all..
that should be handled instead of relying on some OUString (back then
String) magic, which it seems OUString does not silently do and would
crash instead if a length >0 was passed.

Otherwise the patch looks good.

  Eike

-- 
LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD

Attachment: pgpdvBWJhb2Qz.pgp
Description: PGP signature