D18664: Baloo engine: treat every non-success code as a failure

[Igor Poboiko]

poboiko added a comment.


  Nice! I like it, it's definitely much better than `Q_ASSERT_X` macros that are just silently ignored in non-debug builds.
  
  Have some nitpicks though:
  
  1. Are we actually sure this is gonna fix all those crashes? Otherwise I would suggest using CCBUG instead of BUG inside the commit message.
  2. Looks like this patch is composed of two parts - introducing new logging category and and improving error handling. It would be also nice to split those into two separate patches.
  3. For now, logging is the only way to know if there's something bad going on. In that case I would suggest to at least increase severity of those messages - it would increase chances users will notice it. For example, use `qCCritical` (but this would also require additional check for okayish/non-critical return codes, such as `MDB_NOTFOUND`)
  4. There are a lot of redundant `Q_ASSERT_X` left, which could be removed. I suggest just `grep`'ing over the code to catch'em all. I've started marking them here, but then I gave up - too many of them.
  5. There are also several unchecked return codes as well, such as inside `*DB::size()` calls. Those can also be found by `grep`'ing over `Q_ASSERT_X`.

INLINE COMMENTS

> documentdatadb.cpp:142
>          }
>          Q_ASSERT_X(rc == 0, "DocumentDataDB::toTestMap", mdb_strerror(rc));
>  

This is not needed now, I guess

> documentdb.cpp:144
>      MDB_stat stat;
>      int rc = mdb_stat(m_txn, m_dbi, &stat);
>      Q_ASSERT_X(rc == 0, "DocumentDB::size", mdb_strerror(rc));

This can also return something weird.

> postingdb.cpp:132
> +    while (rc == 0) {
>          Q_ASSERT_X(rc == 0, "PostingDB::fetchTermsStartingWith", mdb_strerror(rc));
>  

This assert is not necessary here then?

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D18664

To: valeriymalov, #baloo, bruns, poboiko
Cc: ngraham, bruns, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, abrahams
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190202/40f8fdd6/attachment.html>