Skip to content

Conversation

@hzeller
Copy link
Collaborator

@hzeller hzeller commented Mar 11, 2025

  • std::string_view::data() does not necessarily have a NUL terminated string. So either convert to std::string() first or have function parameters use a const char* to begin with if that is what the caller provides.
  • subtle: the absl cleanup got a copy of the initial allocated buffer pointer, but getline() will modify possibly that value as it might reallocate. So call the cleanup with a reference.

Not an access issue, just simplification:

  • in getting the env var, no need to convert to string_view first, then back to string just to have a nul-terminated string. We call that function with a const char* anyway.

  * std::string_view::data() does not necessarily have a NUL
    terminated string. So either convert to std::string() first
    or have function parameters use a const char* to begin with
    if that is what the caller provides.
  * subtle: the absl cleanup got a _copy_ of the initial allocated
    buffer pointer, but getline() will modify possibly that value as it
    might reallocate. So call the cleanup with a reference.

Not an access issue, just simplification:
  * in getting the env var, no need to convert to string_view first,
    then back to string just to have a nul-terminated string.
    We call that function with a const char* anyway.
@hzeller
Copy link
Collaborator Author

hzeller commented Mar 11, 2025

Thanks to clang-tidy-19 finding these.

Copy link
Owner

@lromor lromor left a comment

Choose a reason for hiding this comment

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

Thanks for catching these issues!

@lromor lromor merged commit 1402a53 into main Mar 11, 2025
4 checks passed
@lromor lromor deleted the feature-20250311-fix-mem branch March 11, 2025 20: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.

3 participants