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

daemon: make ucrednetGet() return *ucrednet #357

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

thp-canonical
Copy link
Contributor

@thp-canonical thp-canonical commented Feb 13, 2024

This ports canonical/snapd#10126 from snapd, so that ucrednetGet() has the same return type.

This change is in preparation for porting the accessChecker API from snapd (canonical/snapd#10127): #358

@thp-canonical thp-canonical force-pushed the ucrednet-sync-with-snapd branch from 0335bad to c96bd43 Compare February 13, 2024 11:29

_ = pid
_ = uid
isUntrusted := (ucred != nil && ucred.Socket == c.d.untrustedSocketPath)
Copy link
Contributor Author

@thp-canonical thp-canonical Feb 13, 2024

Choose a reason for hiding this comment

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

Not sure if this is the right thing (should it be "untrusted" if ucred == nil?), but this is how the same PR looks in snapd (although there it's not called "untrusted" but rather "isSnap": https://github.com/snapcore/snapd/pull/10126/files#diff-6a5ec84e73a526676a7b1e1f5e730211f009679a8736b7e7cda6a111a986ab4cR158

In any case, this is temporary until #358 removes canAccess().

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can just remove the ucred != nil part, as functions that return error like this are almost always guaranteed to never return a nil pointer if err == nil -- this is true of ucrednetGet as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, oops, I missed the fact that we're only handling the err != errNoID case above, but if err == errNoID it could still end up here with a nil ucred. Yeah, I think what you have now is fine.

Copy link
Contributor

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Looks good, but one minor tweak requested.


_ = pid
_ = uid
isUntrusted := (ucred != nil && ucred.Socket == c.d.untrustedSocketPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can just remove the ucred != nil part, as functions that return error like this are almost always guaranteed to never return a nil pointer if err == nil -- this is true of ucrednetGet as well.


_ = pid
_ = uid
isUntrusted := (ucred != nil && ucred.Socket == c.d.untrustedSocketPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, oops, I missed the fact that we're only handling the err != errNoID case above, but if err == errNoID it could still end up here with a nil ucred. Yeah, I think what you have now is fine.

@benhoyt benhoyt merged commit bb59b16 into canonical:master Feb 14, 2024
15 checks passed
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.

2 participants