To comment on the following update, log in, then open the issue:
http://www.openoffice.org/issues/show_bug.cgi?id=112395
User mst changed the following:
What |Old value |New value
================================================================================
CC|'' |'mst'
--------------------------------------------------------------------------------
------- Additional comments from m...@openoffice.org Wed Jun 23 17:43:39 +0000
2010 -------
finally some time to look at your work...
i'm looking at: svarray5_good.patch
(i presume it includes/supercedes all previous patches?)
some specific things:
+#ifndef INCLUDED_VECTOR
+#include <vector>
+#endif
please don't insert external header guards (the #ifndef/#endif around the
include).
you have probably seen some of these in the existing code base, but nowadays
they are obsolete.
last year or so thb ran a script on the entire code to remove them, but the
script was conservative and didn't remove all of them.
+ for ( USHORT i = 0; i < aPortions.size(); i++ )
(several places)
this is wrong: USHORT is the proper index type for SvArrays (they only hold 2^16
entries), but STL vectors use size_t as the index type, and can hold 2^32 (or
even 2^64, depending on implementation) entries.
you have to replace USHORT with size_t, otherwise this can loop infinitely.
postprocess/checkdeliver/checkdeliver.pl
surely this only got in the patch by accident?
+ USHORT GetCount() const { return aEntries.size(); }
same as above: must be size_t, not USHORT (you may need to investigate where
this is called, and update some more types...)
-* SV_DECL_OBJARR(nm, AE, IS, GS)
-* SV_IMPL_OBJARR( nm, AE )
so you've already eliminated one SvArray type? great!
-SV_DECL_PTRARR_DEL( SfxPoolVersionArr_Impl, SfxPoolVersion_Impl*, 0, 2 )
+typedef std::vector<SfxPoolVersion_Impl*> SfxPoolVersionArr_Impl;
i'm not sure what PTRARR_DEL does exactly...
if it does something like call delete on all entries in its destructor, you'll
need to use a vector<boost::shared_ptr> instead.
please check this.
+typedef std::vector<HighlightPortion> HighlightPortions;
it seems this one is duplicate, both in .hxx and .cxx?
#ifndef _SVSTDARR_ULONGS
#define _SVSTDARR_ULONGS
-#include <svl/svstdarr.hxx>
#endif
i guess you can remove the #define _SVSTDARR_ULONGS or similar as well, when you
remove the header (these are to select which SvArrays the header should declare,
and is part of the silliness of SvArrays...)
- ASSERT( pColTbl, "Wo ist meine Color-Tabelle?" );
+ ASSERT( pColTbl, "Where's table color?" );
translating german assertions is a good idea, but in this case it probably
should be "color table", not "table color", because it's actually a lookup table
for colors.
so it seems to me you're definitely going in the right direction here.
i just wonder how big your patch will become...
i'm afraid it will soon be so big that nobody wants to look at it :)
maybe it makes sense to split up the changes somehow?
mabe by SvArray type? maybe by code module?
it should be possible to have several small patches because most of the uses of
the SvArrays are independent of each other.
but one patch for each array will result in thousands of patches, that's also
bad.
do you know about the Mercurial Queues (MQ) extension?
makes managing multiple patches easier...
---------------------------------------------------------------------
Please do not reply to this automatically generated notification from
Issue Tracker. Please log onto the website and enter your comments.
http://qa.openoffice.org/issue_handling/project_issues.html#notification
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@framework.openoffice.org
For additional commands, e-mail: issues-h...@framework.openoffice.org
---------------------------------------------------------------------
To unsubscribe, e-mail: allbugs-unsubscr...@openoffice.org
For additional commands, e-mail: allbugs-h...@openoffice.org
