four changes of suspicious code (connectivity, sc, sw, editeng)

Lionel Elie Mamane lionel at mamane.lu
Fri May 8 04:01:40 PDT 2015


On Fri, May 08, 2015 at 10:23:37AM +0200, Stephan Bergmann wrote:

> One is in connectivity (Lionel, do you happen to know how to trigger that
> code?):

When asking me a question, please CC me (I'm not always very
up-to-date with reading the mailing list). Thanks.

I uploaded a trigger case to
http://people.freedesktop.org/~lmamane/trigger-java_io_Reader::available.odb

Enable macros and run the macro "Main" in "Module1" in that file.

This whole class is a mess. It confuses the notion of byte and the
notion of character. In Java, and also in LibreOffice, a character is
*two* bytes.

It is used to implement the SDBC<->JDBC driver's "getCharacterStream"
methods. From its name, that should return a stream that returns
characters... but no, it returns a XInputStream which is a *byte*
stream. Which makes the difference between getCharacterStream and
getBinaryStream rather ... nonexistent.

The SDBC<->JDBC driver is the only one that implements
getCharacterStream anyway. Most others don't implement
getBinaryStream either, but some do.

This whole sorry situation probably arises from the fact that
LibreOffice's (udk) API does not have character streams, so whoever
"designed" this part of the SDBC API (which is in general a port of
the JDBC API to C++) probably just slapped a binary stream instead.

I'm hesitating between:

1) "Unimplementing" getCharacterStream in the SDBC<->JDBC driver

2) Having it just call getBinaryStream (which uses java_io_InputStream)

3) Fixing java_io_Reader to properly handle the bytes <-> character
   conversion, (which is annoying because it will have to do its own
   buffering for when an odd number of bytes is read...).

> >commit c5e08b42ace5f4481c3db87b4fb6ae2dbf9d9a51
> >Author: Stephan Bergmann <sbergman at redhat.com>
> >Date:   Fri May 8 09:26:35 2015 +0200
> >
> >    I very much assume this wants to call java.io.Reader.ready
> >
> >    ...seeing that there is no java.io.Reader.available.  (And then, there's no good
> >    way to map from java.io.Reader.ready's boolean value to
> >    css.io.XInputStream.available's long value, so conservatively map true to 1.)
> >
> >    But I have no idea how to trigger this code.
> >
> >    Change-Id: I18d12e0d968141410a1b56e700ed544edceda97c
> >
> >diff --git a/connectivity/source/drivers/jdbc/Reader.cxx b/connectivity/source/drivers/jdbc/Reader.cxx
> >index a7cd369..20db510 100644
> >--- a/connectivity/source/drivers/jdbc/Reader.cxx
> >+++ b/connectivity/source/drivers/jdbc/Reader.cxx
> >@@ -57,19 +57,19 @@ void SAL_CALL java_io_Reader::skipBytes( sal_Int32 nBytesToSkip ) throw(::com::s
> >
> > sal_Int32 SAL_CALL java_io_Reader::available(  ) throw(::com::sun::star::io::NotConnectedException, ::com::sun::star::io::IOException, ::com::sun::star::uno::RuntimeException, std::exception)
> > {
> >-    jboolean out(sal_False);
> >+    jboolean out;
> >     SDBThreadAttach t; OSL_ENSURE(t.pEnv,"Java Enviroment geloescht worden!");
> >
> >     {
> >         static const char * cSignature = "()Z";
> >-        static const char * cMethodName = "available";
> >+        static const char * cMethodName = "ready";
> >         // Java-Call
> >         static jmethodID mID(NULL);
> >         obtainMethodId_throwRuntime(t.pEnv, cMethodName,cSignature, mID);
> >         out = t.pEnv->CallBooleanMethod( object, mID);
> >         ThrowRuntimeException(t.pEnv,*this);
> >     } //t.pEnv
> >-    return out;
> >+    return out ? 1 : 0; // no way to tell *how much* is ready
> > }
> >
> > void SAL_CALL java_io_Reader::closeInput(  ) throw(::com::sun::star::io::NotConnectedException, ::com::sun::star::io::IOException, ::com::sun::star::uno::RuntimeException, std::exception)
> 
> One is in sc:
> 
> >commit f593be5bcde09965bb3478e00bcdedbc6bd5bc57
> >Author: Stephan Bergmann <sbergman at redhat.com>
> >Date:   Wed May 6 08:17:32 2015 +0200
> >
> >    SfxBoolItem takes a sal_uInt16 nWhich as first argument
> >
> >    This code was like that ever since 9ae5a91f7955e44d3b24a3f7741f9bca02ac7f24
> >    "initial import."  From the surrounding code, the best bet appears to be
> >    ATTR_LINEBREAK?
> >
> >    Change-Id: Id0e3346f2f9bb9c00c202003d06c2518dea38112
> >
> >diff --git a/sc/source/filter/starcalc/scflt.cxx b/sc/source/filter/starcalc/scflt.cxx
> >index 682eb20..b21f9a4 100644
> >--- a/sc/source/filter/starcalc/scflt.cxx
> >+++ b/sc/source/filter/starcalc/scflt.cxx
> >@@ -1189,7 +1189,7 @@ void Sc10Import::LoadPatternCollection()
> >                     }
> >
> >                 if( ( OJustify & ojWordBreak ) == ojWordBreak )
> >-                    rItemSet.Put( SfxBoolItem( sal_True ) );
> >+                    rItemSet.Put( SfxBoolItem( ATTR_LINEBREAK, true ) );
> >                 if( ( OJustify & ojBottomTop ) == ojBottomTop )
> >                     rItemSet.Put( SfxInt32Item( ATTR_ROTATE_VALUE, 9000 ) );
> >                 else if( ( OJustify & ojTopBottom ) == ojTopBottom )
> >@@ -1830,7 +1830,7 @@ void Sc10Import::LoadColAttr(SCCOL Col, SCTAB Tab)
> >             }
> >
> >             if (OJustify & ojWordBreak)
> >-                aScPattern.GetItemSet().Put(SfxBoolItem(sal_True));
> >+                aScPattern.GetItemSet().Put(SfxBoolItem(ATTR_LINEBREAK, true));
> >             if (OJustify & ojBottomTop)
> >                 aScPattern.GetItemSet().Put(SfxInt32Item(ATTR_ROTATE_VALUE,9000));
> >             else if (OJustify & ojTopBottom)
> 
> One is in sw:
> 
> >commit 2c9b7e3304db9d571d15a4ba3732acf8cf40cffe
> >Author: Stephan Bergmann <sbergman at redhat.com>
> >Date:   Wed May 6 15:16:37 2015 +0200
> >
> >    SvxOpaqueItem takes a sal_uInt16 nId as first argument
> >
> >    This code was like that ever since 84a3db80b4fd66c6854b3135b5f69b61fd828e62
> >    "initial import."  From other uses of SvxOpaqueItem, the best bet appears to be
> >    RES_OPAQUE.
> >
> >    Change-Id: I19de8fac4f41716d9c2a73bda4a8cea200ae99f8
> >
> >diff --git a/sw/source/core/doc/DocumentStylePoolManager.cxx b/sw/source/core/doc/DocumentStylePoolManager.cxx
> >index 3276b9f4..8d4fe0e 100644
> >--- a/sw/source/core/doc/DocumentStylePoolManager.cxx
> >+++ b/sw/source/core/doc/DocumentStylePoolManager.cxx
> >@@ -1318,7 +1318,7 @@ SwFmt* DocumentStylePoolManager::GetFmtFromPool( sal_uInt16 nId )
> >             aSet.Put( SwFmtAnchor( FLY_AT_PAGE ));
> >             aSet.Put( SwFmtHoriOrient( 0, text::HoriOrientation::CENTER, text::RelOrientation::FRAME ));
> >             aSet.Put( SwFmtVertOrient( 0, text::VertOrientation::CENTER, text::RelOrientation::FRAME ));
> >-            aSet.Put( SvxOpaqueItem( sal_False ));
> >+            aSet.Put( SvxOpaqueItem( RES_OPAQUE, false ));
> >             aSet.Put( SwFmtSurround( SURROUND_THROUGHT ));
> >         }
> >         break;
> 
> And one is in editeng:
> 
> >commit 25487875563c5aa05253e3f3199b082ded16184c
> >Author: Stephan Bergmann <sbergman at redhat.com>
> >Date:   Tue May 5 17:47:26 2015 +0200
> >
> >    vcl::Window::Scroll takes a sal_uInt16 nFlags, not a sal_Bool
> >
> >    This passing of sal_True instead of some SCROLL_* flags has been there ever
> >    since fd069bee7e57ad529c3c0974559fd2d84ec3151a "initial import," and the
> >    signature of vcl::Window::Scroll has been like that at least since
> >    f59676fe09175725ee0a75a1678dd1581decc20b "INTEGRATION: CWS
> >    hedaburemove01: #72503# get rid of hedabu procedure: Moving headers to
> >    vcl/inc/vcl and correspondent necessary changes," so lets arbitrarily assume
> >    that this wants to pass SCROLL_CLIP (which has the same numeric value as
> >
> >    Change-Id: I8da6536404aa220611a7df96605c7a4a9efc7f7c
> >
> >diff --git a/editeng/source/editeng/impedit.cxx b/editeng/source/editeng/impedit.cxx
> >index 2061716..3d126ba 100644
> >--- a/editeng/source/editeng/impedit.cxx
> >+++ b/editeng/source/editeng/impedit.cxx
> >@@ -1057,7 +1057,7 @@ Pair ImpEditView::Scroll( long ndX, long ndY, sal_uInt8 nRangeCheck )
> >         aVisDocStartPos = pOutWin->LogicToPixel( aVisDocStartPos );
> >         aVisDocStartPos = pOutWin->PixelToLogic( aVisDocStartPos );
> >         Rectangle aRect( aOutArea );
> >-        pOutWin->Scroll( nRealDiffX, nRealDiffY, aRect, sal_True );
> >+        pOutWin->Scroll( nRealDiffX, nRealDiffY, aRect, SCROLL_CLIP );
> >         pOutWin->Update();
> >         pCrsr->SetPos( pCrsr->GetPos() + Point( nRealDiffX, nRealDiffY ) );
> >         if ( bVisCursor )
> _______________________________________________
> LibreOffice mailing list
> LibreOffice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice


More information about the LibreOffice mailing list