-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fixing SIGSEGV when running containers #303
Conversation
Codecov Report
@@ Coverage Diff @@
## master #303 +/- ##
=======================================
Coverage 48.68% 48.68%
=======================================
Files 185 185
Lines 12168 12168
=======================================
Hits 5924 5924
Misses 5877 5877
Partials 367 367 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
cli/command/container/run.go
Outdated
|
||
if close != nil { | ||
defer close() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the more common way of doing this is to just move the defer close()
after the if err != nil
. This is only nil
if there is an error, so handling the error first will fix the problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dnephin - indeed you are right. i thought that we might have no error and still have nil close.
Thanks for the hint.
@dnephin - LGTY ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Please also squash to a single git commit.
Signed-off-by: Yassine TIJANI <yasstij11@gmail.com> moving the deffering of the close after the error checking Signed-off-by: Yassine TIJANI <yasstij11@gmail.com> fixing SIGSEGV when running containers Signed-off-by: Yassine TIJANI <yasstij11@gmail.com>
384fa20
to
45b0e7c
Compare
@dnephin - Done. |
LGTM |
Signed-off-by: Yassine TIJANI yasstij11@gmail.com
- How I did it checking that close is not before deferring it.
fixes moby/moby#33975