Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node: Fix mask-or operator precedence by replacing + with | #210

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

MarijnS95
Copy link
Contributor

Fixes #209, CC @jbeich

A request for using masks and shifts on the known 0-1-2 constants defining the minor type of a DRM node resulted in the expression to maintain the original + from first subtracting the current node type followed by adding the requested node type. As the subtract of the current node type was replaced by a mask, the resulting old_id & ID_MASK + minor_base expression is interpreted as old_id & (ID_MASK + minor_base) because of operator precedence, while it would have been correctly interpreted as (old_id & ID_MASK) | minor_base when the correct | OR operator is used.

A request for using masks and shifts on the known `0-1-2` constants
defining the minor type of a DRM node resulted in the expression to
maintain the original `+` from first subtracting the current node
type followed by adding the requested node type.  As the subtract of
the current node type was replaced by a mask, the resulting `old_id &
ID_MASK + minor_base` expression is interpreted as `old_id & (ID_MASK
+ minor_base)` because of operator precedence, while it would have been
correctly interpreted as `(old_id & ID_MASK) | minor_base` when the
correct `|` OR operator is used.
Group all this logic together in functions inside `impl NodeType`.
@MarijnS95
Copy link
Contributor Author

CI says:

error: package fdeflate v0.3.6 cannot be built because it requires rustc 1.67.0 or newer, while the currently active rustc version is 1.66.0

Should we bump MSRV, lock fdeflate to an older version, downgrade fdeflate in CI like we already do for home or run the MSRV tests with cargo -nightly -Zminimal-versions generate-lockfile? I prefer the last one because that's what I do in many other crates. It not only exercises -Zminimal-versions which downstream crates/consumers may also be running, but also prevents this error from showing up in the future if any other crate bumps their MSRV beyond our rust-version in a patch release.

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.

Fails to detect DRM nodes on FreeBSD
1 participant