Skip to content

Fixing SCAs - #864

Open
MaciejKaszynski wants to merge 9 commits into
eclipse-score:mainfrom
etas-contrib:SCAs
Open

Fixing SCAs#864
MaciejKaszynski wants to merge 9 commits into
eclipse-score:mainfrom
etas-contrib:SCAs

Conversation

@MaciejKaszynski

@MaciejKaszynski MaciejKaszynski commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Fixing all current codeql findings

cameron-craig-etas
cameron-craig-etas suggested changes Oct 27, 2026
View reviewed changes
Comment thread score/launch_manager/src/daemon/src/alive_monitor/details/ipc/ringbuffer.hpp
// whole T.
// The destination holds sizeof(T) by the static_assert on ElementSize.
// codeql[c/cert/library-function-argument-out-of-bounds] False positive: &value is a whole T
static_cast<void>(std::memcpy(n.storage, &value, sizeof(T)));

@cameron-craig-etas cameron-craig-etas Oct 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could potentially remove this warning by casting &value to const void*/void*.
This also applies to the other similar suppressions.

If memcpy fails, we are treating it as success, that does not sounds good.
If this is ok we should comment why.

Comment thread score/launch_manager/src/daemon/src/alive_monitor/details/ipc/IpcServer.hpp
{
// The object is constructed by std::vector::emplace_back in AliveWorkerFactory::createAliveIfIpc before init()
// is called.
// codeql[cpp/cert/missing-constructor-call-for-manually-managed-object] Constructed by emplace_back

@cameron-craig-etas cameron-craig-etas Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have any guidelines for these suppressions?

I feel like it would be a good idea to differentiate between a false positive and a suppression of a genuine warning? Sounds like this is a false positive, and maybe it should be marked as such?

This branch is waiting to be deployed

1 waiting deployment
workflow-approval — ebae0bad Waiting Oct 27, 2026 by MaciejKaszynski via Build and test x86_64-qnx / approval #1827
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NicolasFussberger NicolasFussberger Awaiting requested review from NicolasFussberger NicolasFussberger is a code owner
@pawelrutkaq pawelrutkaq Awaiting requested review from pawelrutkaq pawelrutkaq is a code owner
@ramceb ramceb Awaiting requested review from ramceb ramceb is a code owner
@FScholPer FScholPer Awaiting requested review from FScholPer FScholPer is a code owner
@antonkri antonkri Awaiting requested review from antonkri antonkri is a code owner
@anmittag anmittag Awaiting requested review from anmittag anmittag is a code owner
1 more reviewer
@cameron-craig-etas cameron-craig-etas cameron-craig-etas requested changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

LCM - Lifecycle & Health FT
Status: Backlog

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MaciejKaszynski @cameron-craig-etas