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

fix nil deference #494

Merged
merged 2 commits into from
Oct 17, 2017
Merged

Conversation

zhouhao3
Copy link

Signed-off-by: zhouhao zhouhao@cn.fujitsu.com

@@ -178,7 +182,7 @@ func validateCapabilities(spec *rspec.Spec) error {
expectedCaps3 := make(map[string]bool)
expectedCaps4 := make(map[string]bool)
expectedCaps5 := make(map[string]bool)
if spec.Process.Capabilities != nil {
if spec.Process != nil && spec.Process.Capabilities != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather move this guard to the top of this function, and have:

if spec.Process == nil || spec.Process.Capabilities == nil {
    return nil
}

I don't see the point to initializing processCaps, etc. when we aren't going to use them.

Copy link
Author

Choose a reason for hiding this comment

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

updated, thanks.

Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
@zhouhao3 zhouhao3 changed the title runtimetest: fix nil deference fix nil deference Oct 12, 2017
Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
@Mashimiao
Copy link

Mashimiao commented Oct 16, 2017

LGTM

Approved with PullApprove

1 similar comment
@liangchenye
Copy link
Member

liangchenye commented Oct 17, 2017

LGTM

Approved with PullApprove

@liangchenye liangchenye merged commit b0e8214 into opencontainers:master Oct 17, 2017
@zhouhao3 zhouhao3 deleted the runtimetest-nil-fix branch October 18, 2017 01:06
wking added a commit to wking/ocitools-v2 that referenced this pull request Jan 12, 2018
6d2dbbc (runtimetest: fix nil deference, 2017-10-11, opencontainers#494) put the
nil-Process guard in, but it was a bit too coarse.  We can still run
these tests with a nil Process, because all we need Process for is
deciding whether or not to include /dev/console.

Signed-off-by: W. Trevor King <wking@tremily.us>
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