Skip to content

Force all Pos* to be non-null#5192

Merged
edolstra merged 2 commits intoNixOS:masterfrom
hercules-ci:non-null-pos
Aug 30, 2021
Merged

Force all Pos* to be non-null#5192
edolstra merged 2 commits intoNixOS:masterfrom
hercules-ci:non-null-pos

Conversation

@roberth
Copy link
Member

@roberth roberth commented Aug 29, 2021

This fixes a class of crashes and introduces ptr<T> to make the
code robust against this failure mode going forward.

Thanks regnat for the idea of a ref<T> without overhead! This became ptr<T>

Closes #4895
Closes #4893
Closes #5127
Closes #5113

Alternatives considered:

This fixes a class of crashes and introduces ptr<T> to make the
code robust against this failure mode going forward.

Thanks regnat for the idea of a ref<T> without overhead!

Closes NixOS#4895
Closes NixOS#4893
Closes NixOS#5127
Closes NixOS#5113
@fogti
Copy link
Contributor

fogti commented Aug 29, 2021

comparisons between positions and noPos are non-uniform. maybe this should also be fixed...

@roberth
Copy link
Member Author

roberth commented Aug 30, 2021

comparisons between positions and noPos are non-uniform. maybe this should also be fixed...

That would be nice, but doing so has the potential to change the behavior. I'd like to keep that to a minimum so this can be merged sooner rather than later.

Details It's just not that simple. `Pos::operator bool()` seems like an obvious candidate, but on closer inspection, flake.cc initializes it with a `file` but `line = 0`. Could it be changed? Yes. Should it? Probably. Does it help with the release? No.

@edolstra edolstra merged commit 00f9957 into NixOS:master Aug 30, 2021
@edolstra
Copy link
Member

@roberth Thanks!

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.

segmentation fault in libnixexpr.so when derivation has no name attribute Nix segfault at evalutation. GC_DONT_GC=1 doesn't help

3 participants