Hi Cedric,

On Thu, 2011-02-10 at 12:08 +0100, Cedric Bosdonnat wrote:
> Could you please review the patch attached to this bug report for
> inclusion in 3.3 and 3.3.1 branches?

        So - I think this is way too big to ship between 3.3.1 RC1 and our
final release of that; so - I'd say no for 3.3.1. I'd say yes for 3.3
(and hence 3.3.2) but I have a few questions:

        When writing we do this:

-    SwWW8Writer::WriteString_xstz( *pDataStrm, ffdeftext, true );
+    if ( !type )
+        SwWW8Writer::WriteString_xstz( *pDataStrm, ffdeftext, true );

-        pDataStream->SeekRel(4 * (nType ? 2 : 1));
+        String sEntryMacro = WW8Read_xstz(*pDataStream, 0, true);
+        String sExitMacro = WW8Read_xstz(*pDataStream, 0, true);
+        //pDataStream->SeekRel(4 * (nType ? 2 : 1));

        That concerns me - we unconditionally read and advance the stream
pointer here presumably by 8 bytes, rather than dependent on nType by
either 4 or 8 (or some multiple thereof). Are we missing an if (nType)
branch here ? and do we have a document that tests the other case ?

        incidentally commenting out code instead of removing it is ...
curious :-)

        Otherwise it looks ok to me.

        Thanks !

                Michael.

-- 
 [email protected]  <><, Pseudo Engineer, itinerant idiot


_______________________________________________
LibreOffice mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Reply via email to