nixos/networkd: fix range assertions on 32 bit Nix #45722
Conversation
| assertInt = name: group: attr: | ||
| optional (attr ? ${name} && !isInt attr.${name}) | ||
| "Systemd ${group} field `${name}' is not an integer"; | ||
|
|
There was a problem hiding this comment.
isInt allows any integer, even negative, so this significantly weakens the assertion too much.
You should keep checking for a range with assertRange but replace the hardcoded upper bound 4294967295 with the largest positive integer depending on the platform's bitsize.
There was a problem hiding this comment.
Why not just checking if it is larger then 0?
There was a problem hiding this comment.
In this particular case there may be a simpler solution: just check for isInt and the lower bound , so you don't have to specify the upper bound.
There was a problem hiding this comment.
(guess we were writing in parallel 😄)
lopsided98
force-pushed
the
networkd-fix-range
branch
from
2d8eb84
to
bdcb5fb
Compare
|
I added checks that the value is >= 0. I also kept the Also, if NixOS/nix#2378 is merged, this PR would no longer be necessary. |
|
Thanks. Nitpick: When the lower bound was 1 before, you should |
lopsided98
force-pushed
the
networkd-fix-range
branch
from
bdcb5fb
to
442681c
Compare
|
Oops, fixed. |
Motivation for this change
Range assertions were recently added to the networkd module that exceeded the range of a signed 32-bit int, breaking the module on 32 bit Nix.
Things done
This PR changes those assertions to checks that the value is an int, as suggested in #44233 (comment).
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)