-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
fd leak and error handling #33713
fd leak and error handling #33713
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,9 @@ func (r *multiReadSeeker) Seek(offset int64, whence int) (int64, error) { | |
rdrOffset := offset - tmpOffset | ||
idx := i | ||
|
||
rdr.Seek(rdrOffset, os.SEEK_SET) | ||
if _, err := rdr.Seek(rdrOffset, os.SEEK_SET); err != nil { | ||
return -1, err | ||
} | ||
// make sure all following readers are at 0 | ||
for _, rdr := range r.readers[i+1:] { | ||
rdr.Seek(0, os.SEEK_SET) | ||
|
@@ -67,7 +69,9 @@ func (r *multiReadSeeker) Seek(offset int64, whence int) (int64, error) { | |
} | ||
tmpOffset += s | ||
} | ||
r.Seek(tmpOffset+offset, os.SEEK_SET) | ||
if _, err := r.Seek(tmpOffset+offset, os.SEEK_SET); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ping @x1022as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aaronlehmann @vieux if this is an important fix, and we want it to be in 17.07, perhaps we should carry to make sure it gets merged in time |
||
return -1, err | ||
} | ||
return tmpOffset + offset, nil | ||
case os.SEEK_CUR: | ||
if r.pos == nil { | ||
|
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.
Does this one need an error check as well?
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 errors should be ignored and continue the for loop, so maybe no need to check here.