Geyslan G. Bem
2013-Oct-11 18:35 UTC
[PATCH v6] btrfs: Fix memory leakage in the tree-log.c
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; -- 1.8.4
Stefan Behrens
2013-Oct-11 18:55 UTC
Re: [PATCH v6] btrfs: Fix memory leakage in the tree-log.c
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> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Geyslan Gregório Bem
2013-Oct-13 22:03 UTC
Re: [PATCH v6] btrfs: Fix memory leakage in the tree-log.c
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? -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Josef Bacik
2013-Oct-14 14:01 UTC
Re: [PATCH v6] btrfs: Fix memory leakage in the tree-log.c
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 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Geyslan Gregório Bem
2013-Oct-14 14:21 UTC
Re: [PATCH v6] btrfs: Fix memory leakage in the tree-log.c
2013/10/14 Josef Bacik <jbacik@fusionio.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, > > JosefThanks to you. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html