D15412: [filters] Extend table lifetime

2018-09-17 Thread Anthony Fieroni
This revision was automatically updated to reflect the committed changes. Closed by commit R8:cb7ff65d2e7c: Extend table lifetime (authored by anthonyfieroni). REPOSITORY R8 Calligra CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15412?vs=41552&id=41841 REVISION DETAIL https://pha

D15412: [filters] Extend table lifetime

2018-09-13 Thread Jarosław Staniek
staniek accepted this revision. staniek added a comment. This revision is now accepted and ready to land. Good job! REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15412 To: anthonyfieroni, danders, boemann, #calligra:_3.0, staniek Cc: staniek, Calligra-Devel-list, dc

D15412: [filters] Extend table lifetime

2018-09-13 Thread Anthony Fieroni
anthonyfieroni updated this revision to Diff 41552. REPOSITORY R8 Calligra CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15412?vs=41377&id=41552 REVISION DETAIL https://phabricator.kde.org/D15412 AFFECTED FILES filters/libmsooxml/MsooXmlDrawingReaderTableImpl.h filters/libmso

D15412: [filters] Extend table lifetime

2018-09-13 Thread Jarosław Staniek
staniek added inline comments. INLINE COMMENTS > anthonyfieroni wrote in MsooXmlDrawingReaderTableMethods.h:40 > That isn't a problem, it's initialized at constructor time. I know but original code was not called new KoTable for PptxXmlSlideReader, right? > anthonyfieroni wrote in DocxXmlDocum

D15412: [filters] Extend table lifetime

2018-09-13 Thread Anthony Fieroni
anthonyfieroni added a comment. It can be allocated only in read_tbl but as you can see in bug report, use-after-free is very common to happen. INLINE COMMENTS > staniek wrote in MsooXmlDrawingReaderTableMethods.h:40 > This is declaration-only header, so better no, otherwise allocation of a

D15412: [filters] Extend table lifetime

2018-09-13 Thread Jarosław Staniek
staniek requested changes to this revision. staniek added a comment. This revision now requires changes to proceed. Thanks, lots of other people also touched this code but I tried this time to review :) I propose to separate the string literal changes. Also I wonder why there's "Context n

D15412: [filters] Extend table lifetime

2018-09-13 Thread Camilla Boemann
boemann added a comment. No idea either - jaroslaw was the original author so maybe he can take a look REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15412 To: anthonyfieroni, danders, boemann, #calligra:_3.0 Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15412: [filters] Extend table lifetime

2018-09-13 Thread Dag Andersen
danders added a comment. Sorry, don't have any qualified opinons, I don't quite understand the original code either, so... Imo if you think it is good, land it. REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15412 To: anthonyfieroni, danders, boemann, #calligra:_3

D15412: [filters] Extend table lifetime

2018-09-12 Thread Anthony Fieroni
anthonyfieroni added a comment. Ping on this, did you try it? REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15412 To: anthonyfieroni, danders, boemann, #calligra:_3.0 Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15412: [filters] Extend table lifetime

2018-09-10 Thread Anthony Fieroni
anthonyfieroni updated this revision to Diff 41377. anthonyfieroni edited the test plan for this revision. anthonyfieroni added a comment. Aggressive refactor, it makes m_table a shared pointer, presetTables is one time initialized. REPOSITORY R8 Calligra CHANGES SINCE LAST UPDATE https:

D15412: [filters] Extend table lifetime

2018-09-10 Thread Anthony Fieroni
anthonyfieroni created this revision. anthonyfieroni added reviewers: danders, boemann, Calligra: 3.0. Herald added a project: Calligra: 3.0. Herald added a subscriber: Calligra-Devel-list. anthonyfieroni requested review of this revision. REVISION SUMMARY CCBUG: 379255 REPOSITORY R8 Calligra