Make name an std::string in OrtMemoryInfo #25960
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR changes the name field in OrtMemoryInfo from a const char* to std::string to prevent dangling pointer issues when users pass arbitrary names that may be deallocated before the API requires them.
- Updates the
OrtMemoryInfostruct to usestd::stringfor the name field instead ofconst char* - Refactors all code that accesses the name field to use
.c_str()when C-string interface is needed - Adds input validation to API functions to prevent null pointer issues
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| include/onnxruntime/core/framework/ortmemoryinfo.h | Changes name field from const char* to std::string and updates comparison operations |
| onnxruntime/core/framework/allocator.cc | Adds input validation and updates API functions to work with string names |
| onnxruntime/core/session/environment.cc | Updates string comparison to use std::string equality |
| onnxruntime/core/framework/bfc_arena.cc | Updates constructor call to use .c_str() |
| onnxruntime/core/session/lora_adapters.cc | Updates string comparisons for memory info names |
| onnxruntime/test/framework/TestAllocatorManager.cc | Updates constructor call to use .c_str() |
| onnxruntime/test/framework/allocator_test.cc | Updates test assertion to use .c_str() |
| onnxruntime/test/framework/tensor_test.cc | Updates test assertions to use .c_str() or direct string comparison |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
snnn
reviewed
Sep 4, 2025
adrianlizarraga
approved these changes
Sep 5, 2025
snnn
pushed a commit
that referenced
this pull request
Sep 8, 2025
### Description <!-- Describe your changes. --> OrtMemoryInfo has a name which is pointer. With the recent changes a user can pass an arbitrary name and then deallocate the string as the API does not require it. Make the name a `std::strng` to own it and refactor. ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> We will have a dangling pointer especially when it comes to other languages. The way it used to work in the past, is that the string would be compared to an internal constant and that constant would be used then. (cherry picked from commit 8ad0614)
snnn
pushed a commit
that referenced
this pull request
Sep 8, 2025
### Description <!-- Describe your changes. --> OrtMemoryInfo has a name which is pointer. With the recent changes a user can pass an arbitrary name and then deallocate the string as the API does not require it. Make the name a `std::strng` to own it and refactor. ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> We will have a dangling pointer especially when it comes to other languages. The way it used to work in the past, is that the string would be compared to an internal constant and that constant would be used then. (cherry picked from commit 8ad0614)
snnn
pushed a commit
that referenced
this pull request
Sep 8, 2025
This PR cherry-picks the following PRs to the rel-1.23.0 branch: * #25938 * #25957 * #25960 * #25968 * #25971 --------- Co-authored-by: Dmitri Smirnov <yuslepukhin@users.noreply.github.com> Co-authored-by: Adrian Lizarraga <adlizarraga@microsoft.com> Co-authored-by: Hariharan Seshadri <shariharan91@gmail.com>
Contributor
|
This PR has been cherry-picked into the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
OrtMemoryInfo has a name which is pointer.
With the recent changes a user can pass an arbitrary name and then deallocate the string
as the API does not require it. Make the name a
std::strngto own it and refactor.Motivation and Context
We will have a dangling pointer especially when it comes to other languages.
The way it used to work in the past, is that the string would be compared to an internal constant and that constant would be used then.