[Libreoffice] [REVIEW] Listbox form fields fixes

Michael Meeks michael.meeks at novell.com
Thu Feb 10 04:33:25 PST 2011


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.

-- 
 michael.meeks at novell.com  <><, Pseudo Engineer, itinerant idiot




More information about the LibreOffice mailing list