Hi,
Attached are 2 patches to covert come SvStrings to std::vector.
I realised I missed some size_t conversions and attached is an updated
version of the second patch (the first patch is fine).
Great stuff, pushed both, thank you! :-)
http://cgit.freedesktop.org/libreoffice/core/commit/?id=d8f2a82f6905178f1f594b22a0d5427b29c8eb33
http://cgit.freedesktop.org/libreoffice/core/commit/?id=a9b3b64a5a94a4c27ac524ac6997ef2e2467267c
I've only read the code, because I was unable to locate the affected
functionality - can you please point me to where exactly should I click
to execute this functionality? ;-)
Good question :-) I traced it back to the AutoText feature in Writer
i.e. Edit -> AutoText. The "Categories" button lets you
create/rename/delete groups and if you highlight some text in the
document before selecting Edit -> AutoText you can create a new entry.
I stumbled on at least one bug that is also present in my distro 3.4.4
build.
* Highlight some text in the main document.
* Edit -> AutoText
* Click "Categories" and create a new group ("TestGroup").
* Under TestGroup create a new entry called "TestEntry" via the
AutoText -> New button.
* Click on the newly created "TestEntry" and then click back on
"TestGroup".
* I believe the AutoText button now incorrectly displays rename/delete
etc.
* Click rename and and a dialog appears for TestEntry and not
TestGroup. Rename TestEntry to TestEntry2 and click Ok.
* The group list is now messed up with the entry name where the group
name should be.
* Keep clicking the various group names and within 10-20s writer will
crash (reproduced numerous times).
I'll open a bug report.
Other than that, aren't we leaking strings in pInsertedArr->erase(it) in
the first patch? But it seems to me that it we were leaking even with
the old code, so I applied that anyway :-) - but would be great if you
can double-check / fix.
Yeah, it looks like all 3 original Remove() calls resulted in leaks. The
attached patch should fix this.
Any comments on the size_t changes/best practice welcomed.
I am not a particular friend of the sal_uInt16 kind of types, but
changing all that to size_t where the size() of the vector might be
returned is counter-productive too, so I think you did the right thing -
casted where necessary.
I realised afterwards that I added some unnecessary casts as comparing
sal_uInt16 and size_t should be fine as they are both unsigned ints e.g.
+ if( static_cast<size_t>(nNewPath) >= m_vPathArr.size() )
I will omit these unnecessary casts in the future to avoid uglifying the
code.
Regards,
Brad
Thank you,
Kendy
>From c1f6ed366747d9d8db12785978db3a8155478afd Mon Sep 17 00:00:00 2001
From: Brad Sowden <c...@sowden.org>
Date: Sat, 31 Dec 2011 11:29:34 +1300
Subject: [PATCH] Fix memory leaks
---
sw/source/ui/misc/glosbib.cxx | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/sw/source/ui/misc/glosbib.cxx b/sw/source/ui/misc/glosbib.cxx
index fff2a2c..3df5186 100644
--- a/sw/source/ui/misc/glosbib.cxx
+++ b/sw/source/ui/misc/glosbib.cxx
@@ -294,6 +294,7 @@ IMPL_LINK( SwGlossaryGroupDlg, DeleteHdl, Button*, pButton )
{
if( **it == sEntry )
{
+ delete *it;
pInsertedArr->erase(it);
bDelete = sal_False;
break;
@@ -310,6 +311,7 @@ IMPL_LINK( SwGlossaryGroupDlg, DeleteHdl, Button*, pButton )
{
if( (*it)->GetToken(0, RENAME_TOKEN_DELIM) == sEntry )
{
+ delete *it;
pRenamedArr->erase(it);
bDelete = sal_False;
break;
@@ -357,6 +359,7 @@ IMPL_LINK( SwGlossaryGroupDlg, RenameHdl, Button *, EMPTYARG )
{
if( **it == sEntry )
{
+ delete *it;
pInsertedArr->erase(it);
pInsertedArr->push_back(new String(sNewName));
bDone = sal_True;
--
1.7.7.4
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
