https://bugs.kde.org/show_bug.cgi?id=433402

Michael Pyne <mp...@kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
      Latest Commit|                            |https://invent.kde.org/fram
                   |                            |eworks/kcoreaddons/commit/c
                   |                            |1f6b6f437cb6764875e1bf67cb8
                   |                            |59f10adcb4d7

--- Comment #2 from Michael Pyne <mp...@kde.org> ---
Git commit c1f6b6f437cb6764875e1bf67cb859f10adcb4d7 by Michael Pyne.
Committed on 23/02/2021 at 00:43.
Pushed by mpyne into branch 'master'.

autotests: Fix autotests to pass under gcc ubsan and leak sanitizer.

The GCC undefined behavior sanitizer (ubsan) flags our deliberate usage
of undefined behavior within the KJob autotests, and the leak sanitizer
flags some memory leaks from the use of KAutoSaveFile.

It would be good to be able to consistently run the autotests under the
use of compiler sanitizers to catch bugs in kcoreaddons libraries, but
this relies on the code still passing the test suite in this case.

It appears to me that the use of undefined behavior in KJobTest is as a
simple sanity check that KJob::isFinished is consistent with the fact
that we just called a slot connected to `KJob::finished()`. But
`KJob::isFinished()` is a protected method and in the context of a
`KJob::finished()` signal emitted from `KJob::~KJob()` there can be no
subclasses to interrogate `KJob::isFinished()` anyways.

I try to go about the sanity check instead by counting specific number
of `finished` calls from each individual job, ensuring that no job ever
emits `finished` more than once, that the corresponding `slotResult`
call (on success) comes after one (and only one) `finished` signal for
the job, and that the number of KJob test that emit `finished` but do
not emit `result` is exactly equal to the number of expected failed jobs

It might be simpler just to use `qobject_cast` or `dynamic_cast` and
only perform the sanity check when we know the `finished` call is
happening from a subclass.

For KAutoSaveFile, it's just a matter of remembering to call delete on
the list of returned KAutoSaveFile* objects. This is worth fixing just
because autotests are sometimes treated as documentation on proper
usage, but I also make sure to clarify in the API documentation (the
code examples in the API docs show correct usage but the per-method
documentation does not make clear that the application owns the returned
pointers).

M  +9    -3    autotests/kautosavefiletest.cpp
M  +16   -8    autotests/kjobtest.cpp
M  +2    -0    autotests/kjobtest.h
M  +10   -0    src/lib/io/kautosavefile.h

https://invent.kde.org/frameworks/kcoreaddons/commit/c1f6b6f437cb6764875e1bf67cb859f10adcb4d7

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to