D9271: Fixed memory leak in new_argv

2019-01-02 Thread Nathaniel Graham
ngraham added a comment. What's the status of this? @nathanhenry are you planning to revive it and address the remaining concerns? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D9271 To: nathanhenry, davidedmundson Cc: ngraham, anthonyfieroni, davidedmundson

D9271: Fixed memory leak in new_argv

2018-11-29 Thread David Edmundson
davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. Many changes remaining - especially in joystick, I'm pretty sure that'll crash. Marking as request changes REPOSITORY R119 Plasma Desktop REVISION DETAIL

D9271: Fixed memory leak in new_argv

2017-12-12 Thread Anthony Fieroni
anthonyfieroni added inline comments. INLINE COMMENTS > Viewer.cpp:143 > viewer=new KFI::CViewer; > viewer->show(); > } You should set attribute delete on close as well. > standard_actions_module.cpp:136 > m_editor->addCollection(m

D9271: Fixed memory leak in new_argv

2017-12-12 Thread Nathan Henry
nathanhenry updated this revision to Diff 23839. nathanhenry added a comment. Fixed m_actionCollection, manager and solved viewer REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9271?vs=23837&id=23839 BRANCH Memory-Leak-Fixes REVISION DETAIL htt

D9271: Fixed memory leak in new_argv

2017-12-12 Thread David Edmundson
davidedmundson added inline comments. INLINE COMMENTS > joydevice.cpp:197 >origCorr = oldCorr; > + delete [] oldCorr; //We should be deleting this when it fails and after > use. >corr = new struct js_corr[axes]; When you do this origCorr is now a dangling pointer to deleted contents.

D9271: Fixed memory leak in new_argv

2017-12-12 Thread Nathan Henry
nathanhenry added inline comments. INLINE COMMENTS > davidedmundson wrote in Viewer.cpp:145 > and then it will crash here. Should this be dereferenced outside the loop or not at all? > davidedmundson wrote in kcm.cpp:83-85 > The runner manager has a parent object. > > http://doc.qt.io/qt-5/obj

D9271: Fixed memory leak in new_argv

2017-12-12 Thread David Edmundson
davidedmundson added inline comments. INLINE COMMENTS > nathanhenry wrote in Viewer.cpp:140-143 > More info? you're deleting an object immediately after show. That will close the dialog > Viewer.cpp:145 > } > viewer->showUrl(url); > + delete viewer

D9271: Fixed memory leak in new_argv

2017-12-12 Thread Nathan Henry
nathanhenry marked 2 inline comments as done. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D9271 To: nathanhenry, davidedmundson Cc: anthonyfieroni, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol,

D9271: Fixed memory leak in new_argv

2017-12-12 Thread Nathan Henry
nathanhenry updated this revision to Diff 23837. nathanhenry added a comment. Fixed indentation and removed new_argv and new_argc REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9271?vs=23692&id=23837 BRANCH Memory-Leak-Fixes REVISION DETAIL htt

D9271: Fixed memory leak in new_argv

2017-12-12 Thread Nathan Henry
nathanhenry added inline comments. INLINE COMMENTS > davidedmundson wrote in Viewer.cpp:140-143 > that can't be right. More info? > davidedmundson wrote in kcm.cpp:83-85 > 1. this doesn't leak > > 2. what's the point of making a manager? How doesn't this leak when excluding my change? REPOSI

D9271: Fixed memory leak in new_argv

2017-12-11 Thread David Edmundson
davidedmundson requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D9271 To: nathanhenry, davidedmundson Cc: anthonyfieroni, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai

D9271: Fixed memory leak in new_argv

2017-12-09 Thread Anthony Fieroni
anthonyfieroni added inline comments. INLINE COMMENTS > main.cpp:1092-1093 > > intnew_argc = 0; > -char **new_argv = new char * [40]; > +char **new_argv = new char * [40]; //Memory Leak > Remove all references to them, they are unused. > Viewer.cpp:146 > vi

D9271: Fixed memory leak in new_argv

2017-12-09 Thread David Edmundson
davidedmundson added inline comments. INLINE COMMENTS > Viewer.cpp:140-143 > if (!first) { > viewer=new KFI::CViewer; > viewer->show(); > + delete viewer; that can't be right. > kcm.cpp:83-85 > Plasma::RunnerManager

D9271: Fixed memory leak in new_argv

2017-12-09 Thread Nathan Henry
nathanhenry updated this revision to Diff 23692. nathanhenry added a comment. Forgot to remove debug statement REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9271?vs=23691&id=23692 BRANCH Memory-Leak-Fixes REVISION DETAIL https://phabricator.kd

D9271: Fixed memory leak in new_argv

2017-12-09 Thread Nathan Henry
nathanhenry created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Fixed memory leak in oldCorr Fixed memory leak Fixed memory leak REPOSITORY R119 Plasma Desktop BRANCH Memory-Leak-Fixes R