From: Josef Bacik <jbacik@fusionio.com>
To: "Geyslan Gregório Bem" <geyslan@gmail.com>
Cc: Stefan Behrens <sbehrens@giantdisaster.de>,
<linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH v6] btrfs: Fix memory leakage in the tree-log.c
Date: Mon, 14 Oct 2013 10:01:33 -0400 [thread overview]
Message-ID: <20131014140133.GA14323@localhost.localdomain> (raw)
In-Reply-To: <CAGG-pURmJKq+w8CNdAEuu8Ajcexfo1mRrKu7TjberKH=1D3rsg@mail.gmail.com>
On Sun, Oct 13, 2013 at 07:03:12PM -0300, Geyslan Gregório Bem wrote:
> 2013/10/11 Stefan Behrens <sbehrens@giantdisaster.de>:
> > On 10/11/2013 20:35, Geyslan G. Bem wrote:
> >>
> >> In add_inode_ref() function:
> >>
> >> Initializes local pointers.
> >>
> >> Reduces the logical condition with the __add_inode_ref() return
> >> value by using only one 'goto out'.
> >>
> >> Centralizes the exiting, ensuring the freeing of all used memory.
> >>
> >> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> >> ---
> >> fs/btrfs/tree-log.c | 33 +++++++++++++++++++--------------
> >> 1 file changed, 19 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> >> index 79f057c..61bb051 100644
> >> --- a/fs/btrfs/tree-log.c
> >> +++ b/fs/btrfs/tree-log.c
> >> @@ -1113,11 +1113,11 @@ static noinline int add_inode_ref(struct
> >> btrfs_trans_handle *trans,
> >> struct extent_buffer *eb, int slot,
> >> struct btrfs_key *key)
> >> {
> >> - struct inode *dir;
> >> - struct inode *inode;
> >> + struct inode *dir = NULL;
> >> + struct inode *inode = NULL;
> >> unsigned long ref_ptr;
> >> unsigned long ref_end;
> >> - char *name;
> >> + char *name = NULL;
> >> int namelen;
> >> int ret;
> >> int search_done = 0;
> >> @@ -1150,13 +1150,15 @@ static noinline int add_inode_ref(struct
> >> btrfs_trans_handle *trans,
> >> * care of the rest
> >> */
> >> dir = read_one_inode(root, parent_objectid);
> >> - if (!dir)
> >> - return -ENOENT;
> >> + if (!dir) {
> >> + ret = -ENOENT;
> >> + goto out;
> >> + }
> >>
> >> inode = read_one_inode(root, inode_objectid);
> >> if (!inode) {
> >> - iput(dir);
> >> - return -EIO;
> >> + ret = -EIO;
> >> + goto out;
> >> }
> >>
> >> while (ref_ptr < ref_end) {
> >> @@ -1169,14 +1171,16 @@ static noinline int add_inode_ref(struct
> >> btrfs_trans_handle *trans,
> >> */
> >> if (!dir)
> >> dir = read_one_inode(root,
> >> parent_objectid);
> >> - if (!dir)
> >> - return -ENOENT;
> >> + if (!dir) {
> >> + ret = -ENOENT;
> >> + goto out;
> >> + }
> >> } else {
> >> ret = ref_get_fields(eb, ref_ptr, &namelen, &name,
> >> &ref_index);
> >> }
> >> if (ret)
> >> - return ret;
> >> + goto out;
> >>
> >> /* if we already have a perfect match, we're done */
> >> if (!inode_in_dir(root, path, btrfs_ino(dir),
> >> btrfs_ino(inode),
> >> @@ -1196,12 +1200,11 @@ static noinline int add_inode_ref(struct
> >> btrfs_trans_handle *trans,
> >> parent_objectid,
> >> ref_index, name,
> >> namelen,
> >> &search_done);
> >> - if (ret == 1) {
> >> - ret = 0;
> >> + if (ret) {
> >> + if (ret == 1)
> >> + ret = 0;
> >> goto out;
> >> }
> >> - if (ret)
> >> - goto out;
> >> }
> >>
> >> /* insert our name */
> >> @@ -1215,6 +1218,7 @@ static noinline int add_inode_ref(struct
> >> btrfs_trans_handle *trans,
> >>
> >> ref_ptr = (unsigned long)(ref_ptr + ref_struct_size) +
> >> namelen;
> >> kfree(name);
> >> + name = NULL;
> >> if (log_ref_ver) {
> >> iput(dir);
> >> dir = NULL;
> >> @@ -1225,6 +1229,7 @@ static noinline int add_inode_ref(struct
> >> btrfs_trans_handle *trans,
> >> ret = overwrite_item(trans, root, path, eb, slot, key);
> >> out:
> >> btrfs_release_path(path);
> >> + kfree(name);
> >> iput(dir);
> >> iput(inode);
> >> return ret;
> >>
> >
> > The patch looks really good now. Thanks!
> >
> > Reviewed-by: Stefan Behrens <sbehrens@giantdisaster.de>
> >
> >
>
> Will it to be committed to the next?
I will pull it into btrfs-next the next time I scrape the list for patches and
then it will go into 3.13. Thanks,
Josef
next prev parent reply other threads:[~2013-10-14 14:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-11 18:35 [PATCH v6] btrfs: Fix memory leakage in the tree-log.c Geyslan G. Bem
2013-10-11 18:55 ` Stefan Behrens
2013-10-13 22:03 ` Geyslan Gregório Bem
2013-10-14 14:01 ` Josef Bacik [this message]
2013-10-14 14:21 ` Geyslan Gregório Bem
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131014140133.GA14323@localhost.localdomain \
--to=jbacik@fusionio.com \
--cc=geyslan@gmail.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=sbehrens@giantdisaster.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).