fix(clang-tidy): resolve cppcoreguidelines-pro-bounds-pointer-arithmetic findings by castler · Pull Request #1013 · eclipse-score/communication

castler · 2026-08-26T08:07:48Z

Summary

Fixes all cppcoreguidelines-pro-bounds-pointer-arithmetic clang-tidy findings in the codebase.

Approach

Replaced raw pointer arithmetic (ptr + n, ptr += n, ptr[n], ptr++) with std::next()-based equivalents. std::next/std::advance perform the actual arithmetic inside the standard library implementation, which is excluded from analysis by HeaderFilterRegex, so this is the idiomatic, standard fix recommended by the C++ Core Guidelines' Bounds Profile for this exact check. All changes are pure syntax transformations with identical runtime semantics — no behavioral change.

Patterns fixed

buf + n / buf[n] → std::next(buf, n) / *std::next(buf, n) — writing at an offset into a raw buffer (tutorial chapters 1/2/3/4/5/6/10/12).
ptr += n → ptr = std::next(ptr, n) — advancing a buffer cursor in a send/receive loop (message_framer.cpp).
argv[i] → *std::next(argv, i) — command-line argument access (consumer.cpp, app1_main.cpp, app2_main.cpp, runtime_configuration.cpp, generic_generic_interaction_app.cpp, generic_typed_interaction_app.cpp).
it++ → it = std::next(it) — raw-pointer-typed container iteration in for-loop increment clauses (application_id_pid_mapping.cpp, transaction_log_set.cpp).
ptr + sizeof(T) → std::next(ptr, sizeof(T)) — computing a byte-range end pointer (message_passing_service_instance.cpp).
ptr + n → std::next(ptr, n) — computing an end-of-buffer pointer (my_bounded_memory_resource.cpp).

Where the distance argument was an unsigned std::size_t, an explicit static_cast<std::ptrdiff_t>(...) was added to avoid introducing a -Wsign-conversion compile error, since std::next's distance parameter is the iterator's (signed) difference_type.

Added #include <iterator> (and <cstddef> where std::ptrdiff_t was newly needed) to each affected translation unit.

Files changed (19)

score/mw/com/doc/tutorial/chapter_{1,2,3,4,5,6,10,12}/... (provider/consumer tutorial samples)
score/mw/com/gateway/transport_layer/sample/message_framer.cpp
score/mw/com/gateway/transport_layer/sample/test/app{1,2}_main.cpp
score/mw/com/impl/bindings/lola/application_id_pid_mapping.cpp
score/mw/com/impl/bindings/lola/messaging/message_passing_service_instance.cpp
score/mw/com/impl/bindings/lola/transaction_log_set.cpp
score/mw/com/runtime_configuration.cpp
score/mw/com/test/generic_skeleton/generic_{generic,typed}_interaction_app.cpp
score/memory/shared/fake/my_bounded_memory_resource.cpp

Verification

Full bazel build //score/mw/com/... //score/memory/... //score/message_passing/... passes.
Scoped clang-tidy build isolated to cppcoreguidelines-pro-bounds-pointer-arithmetic shows 0 findings (down from 26 findings across 12 files).
119 tests pass (1 pre-existing unrelated skip) across score/mw/com/gateway/..., score/mw/com/impl/bindings/lola/..., score/memory/shared/..., score/mw/com/test/generic_skeleton/..., and score/mw/com:runtime_configuration_test.
Tutorial doc targets under score/mw/com/doc/tutorial/... build successfully.

Scope

This PR intentionally contains only cppcoreguidelines-pro-bounds-pointer-arithmetic fixes, per the project convention of one clang-tidy check per PR (see #995, #999, #1000, #1001, #1003, #1012).

    const auto chars_to_copy = std::min(message.size(), capacity - 1U);
    std::memcpy(buf, message.data(), chars_to_copy);
-    buf[chars_to_copy] = '\0';
+    *std::next(buf, chars_to_copy) = '\0';


    const auto chars_to_copy = std::min(message.size(), capacity - 1U);
    std::memcpy(buf, message.data(), chars_to_copy);
-    buf[chars_to_copy] = '\0';
+    *std::next(buf, chars_to_copy) = '\0';


    const auto chars_to_copy = std::min(message.size(), capacity - 1U);
    std::memcpy(buf, message.data(), chars_to_copy);
-    buf[chars_to_copy] = '\0';
+    *std::next(buf, chars_to_copy) = '\0';


    for (int i = 1; i < argc; ++i)
-        if (std::string(argv[i]) == "--mode" && i + 1 < argc)
-            mode = argv[++i];
+        if (std::string(*std::next(argv, i)) == "--mode" && i + 1 < argc)


LittleHuba · 2026-08-26T08:18:19Z

-        mode = argv[1];
+        mode = *std::next(argv, 1);


Not sure whether this makes it better or worse. While it now is modern C++ syntax, the readability of this statement is degraded significantly...

LittleHuba

I'm really unsure whether it makes sense to keep this check.
Yes it does bounds checking in more places but the readibility of our code is degraded so significantly...

…tic findings Replaces raw pointer arithmetic (`ptr + n`, `ptr += n`, `ptr[n]`, `ptr++`) with `std::next()`-based equivalents throughout the codebase, since `<iterator>`'s `std::next`/`std::advance` perform the arithmetic inside the standard library (excluded by HeaderFilterRegex) instead of at the call site, which is the idiomatic fix recommended by the C++ Core Guidelines' Bounds Profile for this exact pattern. All changes are pure syntax transformations with identical runtime semantics - no behavioral change. Patterns fixed: - `buf + n` / `buf[n]` -> `std::next(buf, n)` / `*std::next(buf, n)` for writing at an offset into a raw buffer (tutorial chapters 1/2/3/4/5/6/10/12, gateway sample apps). - `ptr += n` -> `ptr = std::next(ptr, n)` for advancing a buffer cursor in a send/receive loop (message_framer.cpp). - `argv[i]` -> `*std::next(argv, i)` for command-line argument access (consumer.cpp, app1_main.cpp, app2_main.cpp, runtime_configuration.cpp, generic_generic_interaction_app.cpp, generic_typed_interaction_app.cpp). - `it++` -> `it = std::next(it)` for raw-pointer-typed container iteration in for-loop increment clauses (application_id_pid_mapping.cpp, transaction_log_set.cpp). - `ptr + sizeof(T)` -> `std::next(ptr, sizeof(T))` for computing a byte range end pointer (message_passing_service_instance.cpp). - `ptr + n` -> `std::next(ptr, n)` for computing an end-of-buffer pointer (my_bounded_memory_resource.cpp). Where the distance argument was an unsigned `std::size_t`, added an explicit `static_cast<std::ptrdiff_t>(...)` to avoid introducing a `-Wsign-conversion` error, since `std::next`'s distance parameter is the iterator's (signed) `difference_type`. Added `#include <iterator>` (and `<cstddef>` where `std::ptrdiff_t` was newly needed) to each affected translation unit. Verified: - Full build (`bazel build //score/mw/com/... //score/memory/... //score/message_passing/...`) passes. - Scoped clang-tidy build isolated to cppcoreguidelines-pro-bounds-pointer-arithmetic shows 0 findings (down from 26 findings across 12 files). - 119 tests pass (1 pre-existing skip, unrelated) across score/mw/com/gateway/..., score/mw/com/impl/bindings/lola/..., score/memory/shared/..., score/mw/com/test/generic_skeleton/..., and score/mw/com:runtime_configuration_test. - Tutorial doc targets under score/mw/com/doc/tutorial/... build successfully. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…clipse-score#1014) ## Summary Fixes all `cppcoreguidelines-init-variables` clang-tidy findings (11 findings across 4 files). ## Approach Initializes each flagged variable at its point of declaration instead of leaving it default-constructed/uninitialized, per the C++ Core Guidelines' recommendation that every variable be given a well-defined value as soon as it is declared. All changes are pure initialization additions — no behavioral change, since every affected variable is fully overwritten before its first read in the original code, and the chosen initial values (enum default, `0`, `false`) match the variables' subsequent overwritten semantics. ## Fixes - **`unix_domain_engine.cpp`**: `PipeEvent pipe_event` is an out-parameter for a raw `read()` syscall that always overwrites it before use; initialized to `PipeEvent::QUIT` (a valid enumerator) as a safe default. - **`unix_domain_server.cpp`**: `std::uint8_t code` is an out-parameter for `ReceiveProtocolMessage()`; zero-initialized. - **`flock_test.cpp`**: `FILE* stream` was declared then immediately assigned from `fdopen()` on the next line; merged declaration and initialization (×2 occurrences). `int c` (loop-local `fgetc()` result) is zero-initialized. - **`smokeyeyes.cpp`**: the `boost::program_options`-bound variables (`num_clients`, `turns`, `batch_size`, `no_wait`, `num_slots`) are populated by `po::store`/`po::notify` from their `default_value()`s; initialized to matching defaults so the variable is never observably uninitialized between declaration and parsing. `int wstatus`, an out-parameter for `waitpid()`, is zero-initialized. ## Verification - Scoped clang-tidy build isolated to `cppcoreguidelines-init-variables` shows **0 findings** (down from 11 across 4 files). - Full `bazel build //score/message_passing/... //score/mw/com/test/flock/... //score/mw/com/test/smokeyeyes/...` passes. - `bazel test //score/message_passing:unix_domain_test //score/message_passing:unix_domain_test_linux` passes (2/2). - `bazel test //:format_test_C++_with_clang-format` passes — no formatting issues introduced. ## Scope This PR intentionally contains **only** `cppcoreguidelines-init-variables` fixes, per the project convention of one clang-tidy check per PR (see eclipse-score#995, eclipse-score#999, eclipse-score#1000, eclipse-score#1001, eclipse-score#1003, eclipse-score#1012, eclipse-score#1013).

Fixes a formatting violation introduced by the std::next() conversion in the previous commit (line wrap of the std::cerr chain was off by one indentation level). No functional change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ntroduced by std::next() conversion The earlier std::next() conversion (for cppcoreguidelines-pro-bounds-pointer-arithmetic) left the `std::snprintf(std::next(buf, ...), remaining, "%zu", send_counter)` call sites untouched. `std::snprintf` is itself a C-style vararg function, so it now triggers cppcoreguidelines-pro-type-vararg. Since these lines are part of this PR's diff, fix them here too rather than leaving a new finding behind. Replaces the printf-style `std::snprintf` call with `std::to_chars` (C++17, <charconv>), which writes the integer directly without going through a variadic/format-string interface. `std::to_chars` does not null-terminate its output, so a trailing '\0' is written explicitly at the returned pointer (one byte of the original `remaining` budget is reserved for it, matching the space snprintf's `remaining` argument always implied), preserving the "print info to the reader" via `buf` as a null-terminated C-string) as before. Affected: the 5 tutorial provider samples that reference `kHelloWorld` and print a numeric send_counter into the sample buffer (chapters 1, 2, 6, 10, 12). Added `#include <charconv>` and `#include <system_error>` (for `std::errc`); removed the now-unused `#include <cstdio>`. Verified: - Scoped clang-tidy build isolated to cppcoreguidelines-pro-type-vararg over //score/mw/com/doc/tutorial/... shows 0 findings in these 5 files (down from 5; 2 pre-existing findings in unrelated, untouched files remain, out of scope for this PR). - `bazel build //score/mw/com/doc/tutorial/...` passes. - `bazel test //:format_test_C++_with_clang-format` passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

castler · 2026-08-26T13:59:08Z

I'm really unsure whether it makes sense to keep this check. Yes it does bounds checking in more places but the readibility of our code is degraded so significantly...

Not sure if this is really an option. As we are referencing this as some additional measure that we take to achieve MISRA compliance.

communication/quality/static_analysis/misra_gep.md

Line 87 in 6978561

    
           | RULE-8-7-1 | | `-Warray-bounds` <br/> `-Warray-bounds-pointer-arithmetic` | `cppcoreguidelines-pro-bounds-pointer-arithmetic` | `PointerArithmeticFormsAnInvalidPointer.ql`<br/> `PointerArgumentToCstringFunctionIsInvalid.ql` |

…indings (#1015) ## Summary Fixes all `cppcoreguidelines-special-member-functions` clang-tidy findings (3 findings across 3 files). ## Approach Explicitly declares the missing copy/move special member functions (deleted, since none of these classes support meaningful copy/move semantics) so each class fully follows the Rule of Five instead of relying on implicit generation, per the C++ Core Guidelines' recommendation that if any of destructor/copy/move is user-declared, all five should be declared. ## Fixes - **`sample_sender_receiver.cpp`**: `TestDestructor` is a scope-guard type that calls `stop_source_.request_stop()` in its destructor; it is only ever used as a named local variable and was never intended to be copied or moved. Deleted copy/move constructor and assignment. - **`provider.cpp` (partial_restart/provider_restart)**: `CyclicEventSender` already explicitly deleted its copy constructor/assignment (its `std::thread` member isn't copyable) but didn't declare move operations. Since its background thread captures `this`, moving the object would leave the thread referencing a stale instance, so move is deleted too, for the same reason copy already was. - **`smokeyeyes.cpp`**: `SharedState` owns a `pthread_barrier_t` that is placed directly into shared memory via placement-new and destroyed in its destructor; the barrier is tied to this object's address, so copying or moving it is not meaningful. Deleted copy/move constructor and assignment. All changes are pure declarations (`= delete`) with no behavioral change, since none of these types were ever copied or moved elsewhere in the codebase. ## Verification - Scoped clang-tidy build isolated to `cppcoreguidelines-special-member-functions` shows **0 findings** (down from 3 across 3 files). - Full `bazel build //score/mw/com/test/common_test_resources/... //score/mw/com/test/partial_restart/... //score/mw/com/test/smokeyeyes/...` passes. - `bazel test //score/mw/com/test/partial_restart/provider_restart/integration_test:provider_restart_graceful` passes. - `bazel test //:format_test_C++_with_clang-format` passes. ## Scope This PR intentionally contains **only** `cppcoreguidelines-special-member-functions` fixes, per the project convention of one clang-tidy check per PR (see #995, #999, #1000, #1001, #1003, #1012, #1013, #1014).

castler · 2026-08-26T19:27:50Z

Since I will be on leave for now, I do not have the capacity to follow this up. Will do this again when I am back

castler requested review from LittleHuba, bemerybmw, crimson11, hoe-jo and limdor as code owners August 26, 2026 08:07

github-project-automation Bot added this to COM - Communication FT Aug 26, 2026

github-project-automation Bot moved this to Backlog in COM - Communication FT Aug 26, 2026

castler enabled auto-merge August 26, 2026 08:15

github-advanced-security Bot found potential problems Aug 26, 2026

View reviewed changes

LittleHuba reviewed Aug 26, 2026

View reviewed changes

castler marked this pull request as draft August 26, 2026 08:22

auto-merge was automatically disabled August 26, 2026 08:22
Pull request was converted to draft

castler mentioned this pull request Aug 26, 2026

fix(clang-tidy): resolve cppcoreguidelines-init-variables findings #1014

Merged

LittleHuba reviewed Aug 26, 2026

View reviewed changes

castler and others added 2 commits August 26, 2026 11:40

castler mentioned this pull request Aug 26, 2026

fix(clang-tidy): resolve cppcoreguidelines-special-member-functions findings #1015

Merged

castler closed this Aug 26, 2026

github-project-automation Bot moved this from Backlog to Done in COM - Communication FT Aug 26, 2026

Navigation Menu

Repository navigation

fix(clang-tidy): resolve cppcoreguidelines-pro-bounds-pointer-arithmetic findings - #1013

fix(clang-tidy): resolve cppcoreguidelines-pro-bounds-pointer-arithmetic findings#1013
castler wants to merge 3 commits into
eclipse-score:mainfrom
castler:fix/clang-tidy-pro-bounds-pointer-arithmetic

castler commented Aug 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LittleHuba Aug 26, 2026

Uh oh!

LittleHuba left a comment

Uh oh!

castler commented Aug 26, 2026

Uh oh!

castler commented Aug 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

castler commented Aug 26, 2026

Summary

Approach

Patterns fixed

Files changed (19)

Verification

Scope

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LittleHuba Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

LittleHuba left a comment

Choose a reason for hiding this comment

Uh oh!

castler commented Aug 26, 2026

Uh oh!

castler commented Aug 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants