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

Some nits I found from older PRs #4320

Merged
merged 3 commits into from
Jun 13, 2024
Merged

Conversation

kolyshkin
Copy link
Contributor

See commits

1. Do not ask for the same option value twice.

2. For tty, we always want false, unless specified, and this is what
   GetBool gets us.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
An error from strconv.Atoi already contains the text it fails to parse.
Because of that, errors look way too verbose, e.g.:

	[root@kir-rhat runc-tst]# ./runc exec --user 1:1:1 2345 true
	ERRO[0000] exec failed: parsing 1:1 as int for gid failed: strconv.Atoi: parsing "1:1": invalid syntax

With this patch, the error looks like this now:

	[root@kir-rhat runc]# ./runc exec --user 1:1:1 2345 true
	ERRO[0000] exec failed: bad gid: strconv.Atoi: parsing "1:1": invalid syntax

Still not awesome, but better.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@akhilerm
Copy link
Contributor

Will be great if this typo /kering/keyring/ can also be included as part of this patch, https://github.com/opencontainers/runc/blob/main/script/keyring_validate.sh#L76

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Member

@cyphar cyphar left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda AkihiroSuda merged commit ac26d3d into opencontainers:main Jun 13, 2024
39 checks passed
@lifubang lifubang mentioned this pull request Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants