fix panic when Linux is nil #1449
Conversation
|
Instead of having this check everywhere shouldn't we make sure that its non-nil at the beginning? |
|
You mean change spec object after loading config.json depends on whether Linux{} is set or not ? |
|
@Mashimiao validate once after we read the spec and if it is |
|
On 05/17/2017 12:57 AM, Michael Crosby wrote:
@Mashimiao <https://github.com/mashimiao> validate once after we read the spec and if it is |nil| fail before we start executing.
If so, this will be a problem.
The runtime-spec says linux entry is not necessary, just optional even if paltform.os is Linux.
|
|
@Mashimiao oh, ok. Never mind then ;) |
libcontainer/specconv/spec_linux.go
Outdated
| linuxSpecific := false | ||
| if spec.Linux != nil { | ||
| linuxSpecific = true | ||
| } |
There was a problem hiding this comment.
This variable is not necessary since you only use it once. As an aside, the nice way of writing this would be
linuxSpecific := spec.Linux != nil
|
Yes, I noticed this panics recently and hadn't opened an issue yet, it does need fixing... |
|
A simpler solution would be just to set it if nil to the empty config. |
|
Needs a rebase. It would be really nice to fix this as it is a stupid panic and makes testing harder. |
Linux is not always not nil. If Linux is nil, panic will occur. Signed-off-by: Ma Shimiao <[email protected]>
Mashimiao
force-pushed
the
panic-fix-nil-linux
branch
from
7f95893 to
daccd7a
Compare
|
@justincormack rebased. |
|
@Mashimiao please open a new issue with specifically what is still breaking so we can resolve it. We don't have the information here to go on. |
|
OK, I submit a new PR #1559 to fix the problem |
4 participants
Linux is not always not nil.
If Linux is nil, panic will occur.
Signed-off-by: Ma Shimiao [email protected]