D12336: Replace several Q_ASSERTs with proper checks

2019-02-28 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:75db0004ad02: Replace several Q_ASSERTs with proper checks (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12336?vs=33171=52870 REVISION DETAIL

D12336: Replace several Q_ASSERTs with proper checks

2019-02-28 Thread Dominik Haumann
dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I am very much in favour of this change. In fact, we had this discussion already a long time ago on the mailing list, namely that baloo has (had?) zero error handling and simply

D12336: Replace several Q_ASSERTs with proper checks

2019-02-27 Thread Stefan Brüns
bruns edited reviewers, added: ngraham, poboiko; removed: michaelh. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12336 To: bruns, #baloo, #frameworks, ngraham, poboiko Cc: ngraham, apol, sitter, kde-frameworks-devel, broulik, #frameworks, domson, ashaposhnikov,

D12336: Replace several Q_ASSERTs with proper checks

2019-02-26 Thread Stefan Brüns
bruns added a comment. In D12336#333853 , @apol wrote: > If it was silently corrupting the DB maybe the right solution would have been to integrate baloo properly with KCrash. > Like @sitter did in https://phabricator.kde.org/D15573.

D12336: Replace several Q_ASSERTs with proper checks

2018-09-29 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > apol wrote in documenturldb.cpp:44 > Shouldn't asserts help to find where the library is being misused? You gain no information if you crash here. Not the code is wrong, but the processed data. Most importantly, when somebody runs this code,

D12336: Replace several Q_ASSERTs with proper checks

2018-09-29 Thread Aleix Pol Gonzalez
apol added inline comments. INLINE COMMENTS > bruns wrote in documenturldb.cpp:44 > This may be accessed quite frequently, so we would get a lot of warnings, > spamming the journal, and slowing down the system. > > A backtrace is useless here, as the code processes whatever it finds in the >

D12336: Replace several Q_ASSERTs with proper checks

2018-09-29 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > apol wrote in documenturldb.cpp:44 > Would it make sense to include a warning then? If it's a wrong input, we'll > need ways to debug it now that it won't be giving a backtrace. This may be accessed quite frequently, so we would get a lot of

D12336: Replace several Q_ASSERTs with proper checks

2018-09-29 Thread Aleix Pol Gonzalez
apol added subscribers: sitter, apol. apol added a comment. If it was silently corrupting the DB maybe the right solution would have been to integrate baloo properly with KCrash. Like @sitter did in https://phabricator.kde.org/D15573. INLINE COMMENTS > documenturldb.cpp:44 > { > -

D12336: Replace several Q_ASSERTs with proper checks

2018-09-29 Thread Stefan Brüns
bruns added a comment. Herald added a subscriber: kde-frameworks-devel. Ping! REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12336 To: bruns, #baloo, michaelh, #frameworks Cc: kde-frameworks-devel, broulik, #frameworks, ashaposhnikov, michaelh, astippich, spoorun,

D12336: Replace several Q_ASSERTs with proper checks

2018-04-26 Thread Stefan Brüns
bruns marked 2 inline comments as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12336 To: bruns, #baloo, michaelh, #frameworks Cc: broulik, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, bruns

D12336: Replace several Q_ASSERTs with proper checks

2018-04-26 Thread Stefan Brüns
bruns updated this revision to Diff 33171. bruns added a comment. fix variable name REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12336?vs=32529=33171 BRANCH db_robustness2 REVISION DETAIL https://phabricator.kde.org/D12336 AFFECTED FILES

D12336: Replace several Q_ASSERTs with proper checks

2018-04-26 Thread Kai Uwe Broulik
broulik added inline comments. INLINE COMMENTS > documenturldb.cpp:107 > { > -Q_ASSERT(id > 0); > -Q_ASSERT(!name.isEmpty()); > +if (!docId || name.isEmpty()) { > +return; Didn't you mean `id`? > documenturldb.cpp:172 > { > -Q_ASSERT(docId > 0); > +if

D12336: Replace several Q_ASSERTs with proper checks

2018-04-26 Thread Stefan Brüns
bruns added a reviewer: Frameworks. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12336 To: bruns, #baloo, michaelh, #frameworks Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, bruns

D12336: Replace several Q_ASSERTs with proper checks

2018-04-18 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, michaelh. Restricted Application added projects: Frameworks, Baloo. Restricted Application added a subscriber: Frameworks. bruns requested review of this revision. REVISION SUMMARY The code has some preconditions on supplied values when