Skip to content

Conversation

@Ozaq
Copy link
Contributor

@Ozaq Ozaq commented Nov 14, 2025

Description

Fixes a segfault I encountered during profiling, adds additional bounds checking and improves error messages.

Commits:

commit 6cb5d11

Fix potential segfault when reading data

A user defined view may not contain fields of different sizes. Now check
and report errors on size mismatches. Prior this could lead to
overwriting memory and segfaulting subsequently.

commit a3d293e

ChunkedDataView adds bounds check

Assert that we are not writng out of bounds when assembling chunk.

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

🌈🌦️📖🚧 Documentation Z3FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/z3fdb/pull-requests/PR-196

🌈🌦️📖🚧 Documentation FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/fdb/pull-requests/PR-196

@Ozaq Ozaq requested review from simondsmart and tbkr November 14, 2025 13:55
@Ozaq Ozaq force-pushed the fix/improve_cdv_error_messages branch from 6cb5d11 to 18f420c Compare November 14, 2025 13:58
Copy link
Contributor

@tbkr tbkr left a comment

Choose a reason for hiding this comment

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

Not a huge fan of the request passing in the writeInto method but for the sake of readability ok.


/// Writes the extracted data into the out pointer.
/// The caller must ensure there is enought memory allccated for all values to be copied into out.
/// @param request tthat was used to read data from FDB, only used to enhance error messages
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// @param request tthat was used to read data from FDB, only used to enhance error messages
/// @param request that was used to read data from FDB, only used to enhance error messages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2025

Codecov Report

❌ Patch coverage is 0% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.42%. Comparing base (156dd71) to head (2462cf7).

Files with missing lines Patch % Lines
src/chunked_data_view/GribExtractor.cc 0.00% 19 Missing ⚠️
src/chunked_data_view/ViewPart.cc 0.00% 1 Missing ⚠️
tests/chunked_data_view/test_builder.cc 0.00% 1 Missing ⚠️
tests/chunked_data_view/test_view.cc 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #196      +/-   ##
===========================================
- Coverage    72.46%   72.42%   -0.04%     
===========================================
  Files          360      360              
  Lines        21693    21704      +11     
  Branches      2240     2241       +1     
===========================================
  Hits         15719    15719              
- Misses        5974     5985      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Ozaq Ozaq force-pushed the fix/improve_cdv_error_messages branch from 18f420c to 7cd4dcf Compare November 14, 2025 16:43
Ozaq added 2 commits November 14, 2025 17:45
Assert that we are not writng out of bounds when assembling chunk.
A user defined view may not contain fields of different sizes. Now check
and report errors on size mismatches. Prior this could lead to
overwriting memory and segfaulting subsequently.
@Ozaq Ozaq force-pushed the fix/improve_cdv_error_messages branch from 7cd4dcf to 2462cf7 Compare November 14, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants