Skip to content

Conversation

@caolanm
Copy link
Contributor

@caolanm caolanm commented May 17, 2025

  • Resolves: #
  • Target version: master

Summary

TODO

  • ...

Checklist

  • I have run make prettier-write and formatted the code.
  • All commits have Change-Id
  • I have run tests with make check
  • I have issued make run and manually verified that everything looks okay
  • Documentation (manuals or wiki) has been updated or is not required

caolanm added 2 commits May 17, 2025 21:12
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I81a36e992b961ed1b2426c6ee9d6cf8238151520
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I2937e969f2a79e4cc64a52d82f24ed1e3e931ad7
@caolanm
Copy link
Contributor Author

caolanm commented May 18, 2025

coverity flags that a mutex is held to write to ErrorsStream but not to read from it. Adding that looks safe.

Copy link
Contributor

@vmiklos vmiklos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

// output the ErrorsStream we got during the testing
if (!result.wasSuccessful())
{
std::lock_guard<std::mutex> lock(ErrorMutex);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the commit message of 0c46b33, this is indeed used from multiple threads, so then makes sense. First this looked strange, since we clearly ask cppunit to run tests in a suite only on a single thread.

@github-project-automation github-project-automation bot moved this from To Review to To Test in Collabora Online May 19, 2025
@vmiklos vmiklos merged commit 3634c03 into master May 19, 2025
14 checks passed
@vmiklos vmiklos deleted the private/caolan/coverity20250517 branch May 19, 2025 06:57
@github-project-automation github-project-automation bot moved this from To Test to Done in Collabora Online May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants