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

hammer: osd: FileStore: potential memory leak if getattrs fails. #6420

Merged
1 commit merged into from Nov 23, 2015

Conversation

smithfarm
Copy link
Contributor

Memory leak happens if _fgetattrs encounters some error and simply returns.
Fixes: ceph#13597
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>

(cherry picked from commit ace7dd0)
@smithfarm smithfarm self-assigned this Oct 29, 2015
@smithfarm smithfarm added this to the hammer milestone Oct 29, 2015
ghost pushed a commit that referenced this pull request Nov 11, 2015
…s fails.

Reviewed-by: Loic Dachary <ldachary@redhat.com>
ghost pushed a commit that referenced this pull request Nov 13, 2015
…s fails.

Reviewed-by: Loic Dachary <ldachary@redhat.com>
ghost pushed a commit that referenced this pull request Nov 13, 2015
ghost pushed a commit that referenced this pull request Nov 16, 2015
…s fails.

Reviewed-by: Loic Dachary <ldachary@redhat.com>
@@ -3780,6 +3780,7 @@ int FileStore::_fgetattrs(int fd, map<string,bufferptr>& aset)
dout(10) << " -ERANGE, got " << len << dendl;
if (len < 0) {
assert(!m_filestore_fail_eio || len != -EIO);
delete[] names2;
Copy link
Contributor

Choose a reason for hiding this comment

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

May be it's better to use std::unique_ptr to avoid the same issue if code changes?

@smithfarm
Copy link
Contributor Author

@ifed01 Thanks for the review! Please note that this is a backport (to hammer) of ace7dd0

@smithfarm
Copy link
Contributor Author

@xiexingguo Please note the question at #6420 (comment)

ghost pushed a commit that referenced this pull request Nov 19, 2015
…s fails.

Reviewed-by: Loic Dachary <ldachary@redhat.com>
ghost pushed a commit that referenced this pull request Nov 19, 2015
…s fails.

Reviewed-by: Loic Dachary <ldachary@redhat.com>
ghost pushed a commit that referenced this pull request Nov 19, 2015
…s fails.

Reviewed-by: Loic Dachary <ldachary@redhat.com>
@ghost
Copy link

ghost commented Nov 21, 2015

@tchaikov does this backport look good to merge ? It passed a run of the hammer rados suite ( see http://tracker.ceph.com/issues/13356#note-28 for details ).

@ghost ghost assigned tchaikov and unassigned smithfarm Nov 21, 2015
@tchaikov
Copy link
Contributor

@dachary lgtm.

@tchaikov tchaikov assigned ghost and unassigned tchaikov Nov 23, 2015
ghost pushed a commit that referenced this pull request Nov 23, 2015
FileStore: potential memory leak if getattrs fails.

Reviewed-by: Kefu Chai <kchai@redhat.com>
@ghost ghost merged commit d116959 into ceph:hammer Nov 23, 2015
@smithfarm smithfarm deleted the wip-13637-hammer branch November 23, 2015 08:58
@ghost ghost changed the title FileStore: potential memory leak if getattrs fails. hammer: osd: FileStore: potential memory leak if getattrs fails. Feb 18, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants