Conversation
There was a problem hiding this comment.
1 issue found across 38 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/metapack/metapack.cc">
<violation number="1" location="src/metapack/metapack.cc:55">
P2: Writing `extension.size()` with `put_dword` changes this field to little-endian, but readers still parse it as native-endian. This can break metapack parsing on big-endian targets.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
| writer.put_bytes(reinterpret_cast<const std::byte *>(mime.data()), | ||
| mime.size()); | ||
|
|
||
| writer.put_dword(static_cast<std::uint32_t>(extension.size())); |
There was a problem hiding this comment.
P2: Writing extension.size() with put_dword changes this field to little-endian, but readers still parse it as native-endian. This can break metapack parsing on big-endian targets.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/metapack/metapack.cc, line 55:
<comment>Writing `extension.size()` with `put_dword` changes this field to little-endian, but readers still parse it as native-endian. This can break metapack parsing on big-endian targets.</comment>
<file context>
@@ -46,17 +46,17 @@ static auto write_binary_header(std::ostream &output,
+ writer.put_bytes(reinterpret_cast<const std::byte *>(mime.data()),
+ mime.size());
+
+ writer.put_dword(static_cast<std::uint32_t>(extension.size()));
if (!extension.empty()) {
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
</file context>
| writer.put_dword(static_cast<std::uint32_t>(extension.size())); | |
| const auto extension_size{static_cast<std::uint32_t>(extension.size())}; | |
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) | |
| writer.put_bytes(reinterpret_cast<const std::byte *>(&extension_size), | |
| sizeof(extension_size)); |
🤖 Augment PR SummarySummary: This PR upgrades the pinned Changes:
Technical Notes: The new Core I/O module introduces atomic file writing, stream/file-to-string helpers, and a richer set of typed I/O errors that replace some 🤖 Was this summary useful? React with 👍 or 👎 |
| } | ||
| sourcemeta::core::atomic_write_file(path, [&](std::ostream &stream) { | ||
| sourcemeta::core::BinaryWriter writer{stream}; | ||
| writer.put_dword(STATE_MAGIC); |
There was a problem hiding this comment.
src/build/state.cc:1218: BuildState::save now writes header fields via BinaryWriter::put_dword (little-endian), but BuildState::load still reads them with raw memcpy into native uint32_t, so on big-endian systems the state file will be rejected/garbled. Other locations where this applies: src/metapack/metapack.cc:55.
Severity: medium
Other Locations
src/metapack/metapack.cc:55
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Benchmark Index (enterprise)
Details
| Benchmark suite | Current: ebafa92 | Previous: 709f4bd | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
230 ms |
193 ms |
1.19 |
Add one schema (100 existing) |
26 ms |
20 ms |
1.30 |
Add one schema (1000 existing) |
88 ms |
60 ms |
1.47 |
Add one schema (10000 existing) |
919 ms |
772 ms |
1.19 |
Update one schema (1 existing) |
19 ms |
14 ms |
1.36 |
Update one schema (101 existing) |
26 ms |
20 ms |
1.30 |
Update one schema (1001 existing) |
86 ms |
62 ms |
1.39 |
Update one schema (10001 existing) |
717 ms |
521 ms |
1.38 |
Cached rebuild (1 existing) |
6 ms |
4 ms |
1.50 |
Cached rebuild (101 existing) |
8 ms |
5 ms |
1.60 |
Cached rebuild (1001 existing) |
32 ms |
18 ms |
1.78 |
Cached rebuild (10001 existing) |
278 ms |
158 ms |
1.76 |
Index 100 schemas |
136 ms |
105 ms |
1.30 |
Index 1000 schemas |
1049 ms |
835 ms |
1.26 |
Index 10000 schemas |
13502 ms |
11064 ms |
1.22 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Benchmark Index (community)
Details
| Benchmark suite | Current: ebafa92 | Previous: 709f4bd | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
225 ms |
218 ms |
1.03 |
Add one schema (100 existing) |
22 ms |
21 ms |
1.05 |
Add one schema (1000 existing) |
75 ms |
73 ms |
1.03 |
Add one schema (10000 existing) |
909 ms |
616 ms |
1.48 |
Update one schema (1 existing) |
16 ms |
15 ms |
1.07 |
Update one schema (101 existing) |
23 ms |
22 ms |
1.05 |
Update one schema (1001 existing) |
75 ms |
73 ms |
1.03 |
Update one schema (10001 existing) |
636 ms |
997 ms |
0.64 |
Cached rebuild (1 existing) |
4 ms |
4 ms |
1 |
Cached rebuild (101 existing) |
6 ms |
6 ms |
1 |
Cached rebuild (1001 existing) |
26 ms |
25 ms |
1.04 |
Cached rebuild (10001 existing) |
254 ms |
238 ms |
1.07 |
Index 100 schemas |
135 ms |
105 ms |
1.29 |
Index 1000 schemas |
1084 ms |
869 ms |
1.25 |
Index 10000 schemas |
13320 ms |
12747 ms |
1.04 |
This comment was automatically generated by workflow using github-action-benchmark.
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/metapack/metapack.cc">
<violation number="1">
P2: Guard `create_directories(destination.parent_path())` with `has_parent_path()`; calling it with an empty parent path can throw on some standard library implementations.</violation>
<violation number="2">
P1: `assert(!output.fail())` is removed in release builds, so write/open failures can become silent; enable stream exceptions or explicit error checks for all writes.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
| @@ -8,7 +8,8 @@ | |||
| #include <cstring> // std::memcpy | |||
There was a problem hiding this comment.
P1: assert(!output.fail()) is removed in release builds, so write/open failures can become silent; enable stream exceptions or explicit error checks for all writes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/metapack/metapack.cc, line 74:
<comment>`assert(!output.fail())` is removed in release builds, so write/open failures can become silent; enable stream exceptions or explicit error checks for all writes.</comment>
<file context>
@@ -53,44 +47,46 @@ static auto write_binary_header(sourcemeta::core::BinaryWriter &writer,
- }
- });
+ std::ofstream output{destination, std::ios::binary};
+ assert(!output.fail());
+
+ write_binary_header(output, mime, encoding, extension, duration, content,
</file context>
| #include <cstring> // std::memcpy | |
| output.exceptions(std::ofstream::failbit | std::ofstream::badbit); |
Tip: Review your code locally with the cubic CLI to iterate faster.
| @@ -8,7 +8,8 @@ | |||
| #include <cstring> // std::memcpy | |||
There was a problem hiding this comment.
P2: Guard create_directories(destination.parent_path()) with has_parent_path(); calling it with an empty parent path can throw on some standard library implementations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/metapack/metapack.cc, line 100:
<comment>Guard `create_directories(destination.parent_path())` with `has_parent_path()`; calling it with an empty parent path can throw on some standard library implementations.</comment>
<file context>
@@ -101,6 +97,7 @@ auto metapack_write_json(const std::filesystem::path &destination,
const std::chrono::milliseconds duration) -> void {
std::ostringstream buffer;
sourcemeta::core::stringify(document, buffer);
+ std::filesystem::create_directories(destination.parent_path());
write_metapack(destination, mime, encoding, extension, duration,
buffer.str());
</file context>
Signed-off-by: Juan Cruz Viotti jv@jviotti.com