[PUSHED] Re: [PATCH] Converting SV_DECL_PTRARR in the SW module

Michael Stahl mstahl at redhat.com
Wed May 9 00:33:37 PDT 2012


On 08/05/12 10:18, Noel Grandin wrote:
> Hi
> 
> This patch series converts a bunch of SV_DECL_PTRARR in the SW module to 
> boost::ptr_vector and std::vector variously.
> 
> It passes "make sw.check" and "make check"
> 
> (In the interests of full disclosure I should say that my machine 
> appears to have some issues, what I mean is that I get the same number 
> of errors in "make check" both with and without these patches)

thanks, pushed the whole lot to master, with following caveats:

patch #2 had some un-converted OSL_ENSUREs.

same for patches #11 #13 #14 #15 #18; _perhaps_ it would be a good idea
to test your patches with "make debug=t" or --enable-dbutil before
submitting them  :)

patch #16:
> I had to move "struct SwHTMLFmtInfo" from htmlatr.cxx to
> wrthtml.hxx because there are clear() calls in wrthtml.cxx
> and ptr_set needs the full declaration to be visible.

in this case i would have preferred to use a
std::set<boost::shared_ptr>, which doesn't have this problem; it is
always good to keep headers as small as possible.

patches #11 #27 #32 had this problem:

std::vector::vector(size_type n) creates a vector containing n default
initialized elements, while boost::ptr_vector or SvArray constructors
create a vector containing 0 elements and allocate space for n.

because a lot of this old code is hyper-micro-optimized, it is not
uncommon to see a "FooBarArr foo(17);" somewhere (where the number was
almost certainly determined by fair dice roll), which does something
completely different with STL containers.

also, the compiler won't find these bugs for you, unless you rename the
types (which may be a good idea in some cases anyway, e.g. when they
have that horrible "_" prefix).



More information about the LibreOffice mailing list