Skip to content

fix(libutil): avoid dangling reference warning in Hash deserialization#14349

Closed
lovesegfault wants to merge 1 commit intoNixOS:masterfrom
lovesegfault:fix-hash-warn
Closed

fix(libutil): avoid dangling reference warning in Hash deserialization#14349
lovesegfault wants to merge 1 commit intoNixOS:masterfrom
lovesegfault:fix-hash-warn

Conversation

@lovesegfault
Copy link
Member

Motivation

Changed the hashS variable from a reference to a copy to eliminate the
-Wdangling-reference compiler warning.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Changed the hashS variable from a reference to a copy to eliminate the
-Wdangling-reference compiler warning.
@Ericson2314
Copy link
Member

Ah I would like to fix valueAt, whose returning by value is the underlying problem. (getString already does the right thing.)

auto & obj = getObject(json);
auto algo = parseHashAlgo(getString(valueAt(obj, "algorithm")), xpSettings);
auto format = parseHashFormat(getString(valueAt(obj, "format")));
auto & hashS = getString(valueAt(obj, "hash"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Here it's fine actually. The warning is a false-positive caused by the second argument being a temporary std::string that dies at the end of the full expression. I think this is an instance of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114220.

Copy link
Member

Choose a reason for hiding this comment

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

I will try to avoid that by fixing valueAt though.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can annotate it with [[gnu::no_dangle]] to silence the warning and remove the overload that takes a json && to make it fool-proof.

@xokdvium
Copy link
Contributor

What we should do instead is mark valueAt with [[gnu::no_dangle]] to silence these false-positives

@Ericson2314
Copy link
Member

Ericson2314 commented Oct 24, 2025

#14351 Here is the PR doing the thing I mentioned, which I think side-steps the issue completely, as well as making things more efficient.

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