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

Adjust Process.egid= to accept Strings #2669

Merged
merged 8 commits into from
Jun 17, 2022

Conversation

ngtban
Copy link
Contributor

@ngtban ngtban commented Jun 2, 2022

@ngtban ngtban marked this pull request as draft June 2, 2022 13:53
@ngtban ngtban changed the title 2615 proc set egid 2615 Adjust Process.egid= to accept Strings Jun 2, 2022
@ngtban ngtban changed the title 2615 Adjust Process.egid= to accept Strings Adjust Process.egid= to accept Strings Jun 2, 2022
@ngtban ngtban marked this pull request as ready for review June 2, 2022 14:30
@oracle oracle deleted a comment from bjfish Jun 10, 2022
@eregon
Copy link
Member

eregon commented Jun 10, 2022

Could you also add a changelog entry? (see CONTRIBUTING.md)

@ngtban
Copy link
Contributor Author

ngtban commented Jun 11, 2022

@bjfish, @eregon Sorry for the late reply, in part I got unexpectedly busy this week, in part I got lost in trying to understand the C source for Truffle::Type.rb_num2uint. I will adjust the PR.

@ngtban ngtban requested a review from eregon June 16, 2022 04:13
as_superuser do
context "when ran by a superuser" do
it "sets the effective group id for the current process if run by a superuser" do
Process.egid = 1
Copy link
Member

Choose a reason for hiding this comment

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

This seems way too dangerous as a side-effect, could you restore the original egid in ensure?

Copy link
Member

Choose a reason for hiding this comment

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

I'll do it, to avoid another feedback cycle.

@eregon eregon added the in-ci The PR is being tested in CI. Do not push new commits. label Jun 16, 2022
@eregon
Copy link
Member

eregon commented Jun 16, 2022

Thank you for the PR, I'll submit it to the merge queue now.

@eregon eregon force-pushed the 2615-proc-set-egid branch from dfc920f to fe1a8ef Compare June 16, 2022 20:27
@graalvmbot graalvmbot merged commit 242e7a1 into oracle:master Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-ci The PR is being tested in CI. Do not push new commits. oca-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants