Hi Ryan, Thanks for your patch! On Wednesday, 2015-07-29 22:45:18 +1200, Ryan McCoskrie wrote:
(I'm having trouble with logerrit)
That is why? We much prefer patches on gerrit, it eases review and handling a lot. Using git review might also be an option for you. If for some reason you *have* to use mail attachments, please use git format-patch to generate the patch to be attached, which preserves author and date and commit message, or even use the git send-email command. That being said ...
diff --git a/scaddins/source/datefunc/datefunc.cxx b/scaddins/source/datefunc/datefunc.cxx ScaFuncDataList::ScaFuncDataList( ResMgr& rResMgr ) : - nLast( 0xFFFFFFFF ) + nLast( contents.begin() )
This looks odd. I think it should be contents.end() instead. But why change it to iterator anyway? When using a vector, the last accessed name could still be remembered as offset into the vector.
ScaFuncDataList::~ScaFuncDataList()
{
- for( ScaFuncData* pFData = First(); pFData; pFData = Next() )
- delete pFData;
+ for( std::vector<ScaFuncData*>::iterator it = contents.begin(); it != contents.end(); ++it )
+ delete *nLast;
This does not work.. it deletes *nLast multiple times. It should be *it instead.
-const ScaFuncData* ScaFuncDataList::Get( const OUString& rProgrammaticName ) const
+const ScaFuncData* ScaFuncDataList::Get( const OUString& rProgrammaticName )
{
if( aLastName == rProgrammaticName )
- return Get( nLast );
+ return *nLast;
So this could be return contents[nLast]; if nLast was still the offset intead of an iterator.
diff --git a/scaddins/source/datefunc/datefunc.hxx b/scaddins/source/datefunc/datefunc.hxx
-class ScaFuncDataList : private ScaList
+class ScaFuncDataList
{
- OUString aLastName;
- sal_uInt32 nLast;
+ OUString aLastName;
+ std::vector<ScaFuncData*>::iterator nLast;
Please adapt the variable name when changing the type, we use the 'n' prefix for numeric variables, it then should be itLast or some such. However, as said, size_t nLast would still be fine.
+ std::vector<ScaFuncData*> contents;
Maybe naming it vContents would be better.
- inline const ScaFuncData* Get( sal_uInt32 nIndex ) const; - const ScaFuncData* Get( const OUString& rProgrammaticName ) const; + const ScaFuncData* Get( const OUString& rProgrammaticName );
Instead of making the function non-const you could also make nLast and aLastName mutable member variables. All above the same for the pricing Add-In.
diff --git a/scaddins/source/pricing/pricing.cxx b/scaddins/source/pricing/pricing.cxx
ScaFuncDataList::~ScaFuncDataList()
{
- for( ScaFuncData* pFData = First(); pFData; pFData = Next() )
- delete pFData;
+ for( std::vector<ScaFuncData*>::iterator it = contents.begin(); it != contents.end(); ++it )
+ delete *it;
This one correctly deletes *it. Last but not least: Apparently we don't have your license statement on file, could you please send us a blanket statement that you contribute all your past and future patches under the MPLv2 and LGPLv3+ licenses? Best on the dev mailing list libreoffice@lists.freedesktop.org so we can link to it from https://wiki.documentfoundation.org/Development/Developers Something like this does nicely: All of my past & future contributions to LibreOffice may be licensed under the MPLv2/LGPLv3+ dual license. Best use Subject: <your full name> license statement Sorry for the inconvenience and thank you for cooperating :-) Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GPG key "ID" 0x65632D3A - 2265 D7F3 A7B0 95CC 3918 630B 6A6C D5B7 6563 2D3A Better use 64-bit 0x6A6CD5B765632D3A here is why: https://evil32.com/ Care about Free Software, support the FSFE https://fsfe.org/support/?erack
Attachment:
pgpFaRpSIaia7.pgp
Description: PGP signature