Avoid manual memory management of rd_file_kw by eivindjahren · Pull Request #1243 · equinor/resdata

eivindjahren · 2026-07-08T11:21:55Z

No description provided.

Copilot

Pull request overview

This PR refactors the rd_file_kw/rd_file_view internals to avoid manual lifetime management of keyword wrappers by introducing a C++ FileKW class (smart-pointer based) and updating call sites to use it, while removing the old transaction-based unloading mechanism.

Changes:

Replace rd_file_kw_type + transaction ref-counting with FileKW and a new rd_file_view_clear()-based unloading approach.
Update rd_file_view to store keywords as std::shared_ptr<FileKW> and children as std::unique_ptr, and adjust loaders/users accordingly.
Update/trim tests and CMake targets to reflect the new API (including removing the legacy test_transactions).

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
lib/tests/test_well_info.cpp	Removes a lazy-loading test (suggest re-adding coverage for new clear-based unloading).
lib/tests/test_rd_file_kw.cpp	Updates test to use `FileKW` wrapper and method-based write path.
lib/resdata/well_info.cpp	Replaces transaction guard with `rd_file_view_clear()` guard between UNRST blocks.
lib/resdata/tests/test_transactions.cpp	Removes legacy transaction test executable.
lib/resdata/tests/rd_file_view.cpp	Updates tests to `FileKW` but drops read/round-trip coverage (should be restored).
lib/resdata/rd_unsmry_loader.cpp	Updates keyword header comparisons to use `FileKW::get_header()`.
lib/resdata/rd_file.cpp	Switches to `inv_map_type` allocation via `new/delete`, uses `FileKW` and `shared_ptr` in scan/save flows.
lib/resdata/rd_file_view.cpp	Migrates view storage to `shared_ptr<FileKW>`, removes transactions, adds `rd_file_view_clear()`, updates indexing/IO.
lib/resdata/rd_file_kw.cpp	Introduces `FileKW` implementation including serialization and lazy loading.
lib/include/resdata/rd_file_view.hpp	Updates public API: adds `rd_file_view_clear`, moves `iget_file_kw/add_kw` to C++-only section, defines `inv_map_type`.
lib/include/resdata/rd_file_kw.hpp	Adds `FileKW` class declaration; removes legacy C-style API.
lib/CMakeLists.txt	Removes `test_transactions` from the build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

This moves the responsibility of updating inv_map to file_view, which enables us to drop cleared keywords from inv_map. It also changes the memory management of file_list to use shared_ptr. Before the memory management would be done by choosing one file_view to own the references while other might get a non-owning reference. This avoids this brittle pattern by just using a shared_ptr.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

This also changes the checked_alloc so that it correctly handles the num==0 case which where std::(re|m|c)alloc has implementation-defined behavior: either return nullptr or a memory segment that cannot be accessed but must be freed. By returning nullptr, if the caller handled the num==0 correctly, they can used the checked version.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

eivindjahren requested a review from Copilot July 8, 2026 11:21

Copilot started reviewing on behalf of eivindjahren July 8, 2026 11:22 View session

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch from f939881 to 7133723 Compare July 8, 2026 11:29

Copilot AI reviewed Jul 8, 2026

View reviewed changes

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch 4 times, most recently from 1841d8f to 2d27fc2 Compare July 8, 2026 12:20

eivindjahren requested a review from Copilot July 8, 2026 12:22

Copilot started reviewing on behalf of eivindjahren July 8, 2026 12:23 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

Comment thread lib/include/resdata/rd_file_kw.hpp Outdated

Comment thread lib/resdata/rd_file_view.cpp Outdated

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch 2 times, most recently from a98657e to fcf70db Compare July 8, 2026 12:32

eivindjahren requested a review from Copilot July 8, 2026 12:36

Copilot started reviewing on behalf of eivindjahren July 8, 2026 12:36 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

eivindjahren changed the title ~~Avoid manual memory of rd_file_kw~~ Avoid manual memory management of rd_file_kw Jul 9, 2026

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch from fcf70db to bb9da9e Compare July 9, 2026 04:53

eivindjahren requested a review from Copilot July 9, 2026 04:53

Copilot started reviewing on behalf of eivindjahren July 9, 2026 04:54 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread lib/include/resdata/rd_file_kw.hpp Outdated

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch from 3175e70 to 39e15d3 Compare July 9, 2026 05:29

eivindjahren requested a review from Copilot July 9, 2026 05:29

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch from 39e15d3 to 591cd9f Compare July 9, 2026 05:30

Copilot started reviewing on behalf of eivindjahren July 9, 2026 05:30 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread lib/resdata/rd_file_kw.cpp Outdated

Comment thread lib/resdata/rd_file.cpp

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch from 591cd9f to 010b4fc Compare July 9, 2026 05:51

eivindjahren requested a review from Copilot July 9, 2026 05:51

Copilot started reviewing on behalf of eivindjahren July 9, 2026 05:51 View session

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch from 010b4fc to 2080575 Compare July 9, 2026 05:55

eivindjahren added 6 commits July 9, 2026 10:20

Use constructors for file_kw instead of alloc

4b7b023

Use operator== for rd_file_kw

0f532f5

Use unordered map for inv_map

0866ae9

Replace the transaction concept with clear

ced3a1f

Use unique_ptr for child_list

987d249

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch 2 times, most recently from fb673fb to 444b773 Compare July 9, 2026 08:33

eivindjahren added 3 commits July 9, 2026 10:44

Replace old style tests for ResdataKW with catch equivalent

611675c

Use rd::checked_malloc for FileKW::read

5fa3bb8

Validate number of keywords to read

1563629

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch from 444b773 to 20c39ef Compare July 9, 2026 08:44

eivindjahren requested a review from Copilot July 9, 2026 08:45

Copilot started reviewing on behalf of eivindjahren July 9, 2026 08:45 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread lib/include/resdata/rd_util.hpp

Comment thread lib/include/resdata/rd_util.hpp

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch 3 times, most recently from 3e171dd to 9974cf4 Compare July 9, 2026 09:33

eivindjahren requested a review from Copilot July 9, 2026 09:33

Copilot started reviewing on behalf of eivindjahren July 9, 2026 09:34 View session

eivindjahren added 3 commits July 9, 2026 11:34

Guard against overflow of buffer_size

26927a5

Simplify loop in FileKW::read

c5ca1e4

eivindjahren force-pushed the fix_manual_memory_in_rd_file_view branch from 9974cf4 to c5ca1e4 Compare July 9, 2026 09:36

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread lib/include/resdata/rd_file_view.hpp

Comment thread lib/include/resdata/rd_file_kw.hpp

eivindjahren requested a review from Copilot July 9, 2026 09:40

Copilot started reviewing on behalf of eivindjahren July 9, 2026 09:41 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

eivindjahren merged commit fa7f6fb into main Jul 9, 2026
11 checks passed

eivindjahren deleted the fix_manual_memory_in_rd_file_view branch July 9, 2026 09:53

Uh oh!

Conversation

eivindjahren commented Jul 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants