Add nil check of spec.Process in validateProcessSpec() #2413
Conversation
kolyshkin
changed the title
|
@JFHwang thank you for your contribution! I have modified the PR subject and description a bit. Can you please change the commit description to say not what it's doing (as it's very clear from the patch itself) but why are you doing it, or what is the resulting effect? Something like "validateProcessSpec: prevent SEGV with invalid config". |
utils_linux.go
Outdated
| @@ -378,6 +378,9 @@ func (r *runner) checkTerminal(config *specs.Process) error { | |||
| } | |||
|
|
|||
| func validateProcessSpec(spec *specs.Process) error { | |||
| if spec == nil { | |||
| return fmt.Errorf("process property must not be empty") | |||
There was a problem hiding this comment.
nit: since there's no formatting, it's better to use errrors.New().
You can add a patch just before this one, converting the other places like this.
There was a problem hiding this comment.
Okay, will do. Thanks for the advice!
There was a problem hiding this comment.
Hi, I'm a bit unsure what is meant by a patch. Does this relate to git's patch files? A separate issue? A separate commit? Or just another change I apply to the current commit?
Thanks!
There was a problem hiding this comment.
I mean two separate commits in this same PR. First one doing cleanup replacing fmt.Errorf() to errors.New() in this file in cases there are no %-style formatting. Second is what you did.
JFHwang
force-pushed
the
2392-spec-check
branch
3 times, most recently
from
e021f83 to
d20605d
Compare
Signed-off-by: John Hwang <[email protected]>
|
LGTM |
|
@JFHwang you forgot to use otherwise LGTM |
…lid. Signed-off-by: John Hwang <[email protected]>
|
LGTM |
1 similar comment
|
LGTM |
The compilation error had ocurred because of a bad rebase during opencontainers#2401 and opencontainers#2413 Signed-off-by: Akihiro Suda <[email protected]>
Signed-off-by: John Hwang [email protected]
Short 3 line nil check to ensure spec.Process exists before validating its contents.
Closes #2392