Re: [Libreoffice] [REVIEW] request for review for patch for fdo#32742# regression

On Wed, Jan 12, 2011 at 10:35 AM, Octavio Alvarez
<alvarezp@alvarezp.ods.org> wrote:
> On Wed, 12 Jan 2011 03:18:01 -0800, Noel Power <nopower@novell.com> wrote:
>
> > Hi there
> >
> > fix for fdo#32742#, its a simple fix ( but was a bugger to find ) imo is
> > riskless etc.
> >
> >
> > http://cgit.freedesktop.org/libreoffice/base/commit/?id=0afe2266016b03f6e11008463042c7daacead0e1
> > is ripe for signoff and cherrypicking, be grateful if someone could help
> > with that
>
> Is suggest to make ::std::sort on the next line use s_nCount also and
> prevent a useless division operation.

Actually the 'useless' division' is resolved at compile time. so it is free.
on the other hand passing s_nCount instead of a constant _could_
deprive the template code of some possible optimizations.
(I don't know if std::sort is smart enough to take avantage of it..
but it could, and eventually it will)

Norbert

> For that, consider the patch 72cc77c8bb that introduced the regression.
> The correct patch 72cc77 should have just changed the first s_nCount line.
>
> diff --git a/reportdesign/source/ui/inspection/metadata.cxx
> b/reportdesign/source/ui/inspection/metadata.cxx
> index db219c8..e3610c4 100644
> --- a/reportdesign/source/ui/inspection/metadata.cxx
> +++ b/reportdesign/source/ui/inspection/metadata.cxx
> @@ -173,9 +173,8 @@ namespace rptui
>           };
>
>           s_pPropertyInfos = aPropertyInfos;
> -        s_nCount = sizeof(aPropertyInfos) / sizeof(OPropertyInfoImpl);
>
> -        ::std::sort( aPropertyInfos, aPropertyInfos + s_nCount,
> PropertyInfoLessByName() );
> +        ::std::sort( aPropertyInfos, aPropertyInfos +
> SAL_N_ELEMENTS(aPropertyInfos), PropertyInfoLessByName() );
>
>           return s_pPropertyInfos;
>       }
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice