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

Refactor sysfs to improve sanitization - Part 3 #360

Merged

Conversation

vijaydhanraj
Copy link
Contributor

@vijaydhanraj vijaydhanraj commented Jan 25, 2022

Description of the changes

This PR cleans up a few of the TODOs as part of refactoring sysfs.

How to test this PR?

Please run sysfs_common and proc_common regression tests.


This change is Reviewable

dimakuv
dimakuv previously approved these changes Jan 25, 2022
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @vijaydhanraj)


-- commits, line 7 at r1:
and unify (you forgot and)


-- commits, line 8 at r1:
PAL -> PALs

@dimakuv
Copy link

dimakuv commented Jan 25, 2022

Jenkins, test this please

Copy link
Contributor Author

@vijaydhanraj vijaydhanraj left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (3 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)


-- commits, line 6 at r1:
Also, fixed Linux_SGX->Linux-SGX


-- commits, line 7 at r1:

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

and unify (you forgot and)

yes, done.


-- commits, line 8 at r1:

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

PAL -> PALs

Done.


-- commits, line 8 at r1:
Fixed typo, direcly -> directly

dimakuv
dimakuv previously approved these changes Jan 25, 2022
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

@dimakuv
Copy link

dimakuv commented Jan 25, 2022

Jenkins, test this pleae

@dimakuv
Copy link

dimakuv commented Jan 25, 2022

Jenkins, test this please

mkow
mkow previously approved these changes Jan 25, 2022
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required)

boryspoplawski
boryspoplawski previously approved these changes Jan 25, 2022
Copy link
Contributor

@boryspoplawski boryspoplawski left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions

This function was introduced to copy topology information from
`g_pal_sec` struct to `g_pal_public_state` for Linux-SGX PAL. Given
that `pal_sec` struct was removed, we can remove this function and
unify the behavior between Linux and Linux-SGX PALs to directly copy
topology information to `g_pal_public_state`.

Signed-off-by: Vijay Dhanraj <vijay.dhanraj@intel.com>
Signed-off-by: Vijay Dhanraj <vijay.dhanraj@intel.com>
@vijaydhanraj vijaydhanraj dismissed stale reviews from boryspoplawski, mkow, and dimakuv via 79efb2f January 25, 2022 23:43
@vijaydhanraj vijaydhanraj force-pushed the vdhanraj/refactor_sysfs_part3 branch from e820bb0 to 79efb2f Compare January 25, 2022 23:43
@mkow
Copy link
Member

mkow commented Jan 25, 2022

Jenkins, retest this please

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL)

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)

Copy link
Contributor

@boryspoplawski boryspoplawski left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@dimakuv dimakuv merged commit 79efb2f into gramineproject:master Jan 26, 2022
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.

4 participants