diff mbox

Btrfs: add missing error check to find_parent_nodes

Message ID 1375210462-17820-1-git-send-email-fdmanana@gmail.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Filipe Manana July 30, 2013, 6:54 p.m. UTC 
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 fs/btrfs/backref.c |    2 ++
 1 file changed, 2 insertions(+)

Comments

Zach Brown July 30, 2013, 11 p.m. UTC | #1 
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 8bc5e8c..1ba87c5 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -935,6 +935,8 @@ again:
>  				}
>  				ret = find_extent_in_eb(eb, bytenr,
>  							*extent_item_pos, &eie);
> +				if (ret < 0)
> +					goto out;
>  				ref->inode_list = eie;
>  				free_extent_buffer(eb);
>  			}

The eb needs to be freed before the goto.

And that there's now *three* eb freeing paths in this block should be a
hint that it should be refactored into a function that frees the eb
before returning.

- z
--
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
Filipe Manana July 30, 2013, 11:27 p.m. UTC | #2 
On Wed, Jul 31, 2013 at 12:00 AM, Zach Brown <zab@redhat.com> wrote:
>> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
>> index 8bc5e8c..1ba87c5 100644
>> --- a/fs/btrfs/backref.c
>> +++ b/fs/btrfs/backref.c
>> @@ -935,6 +935,8 @@ again:
>>                               }
>>                               ret = find_extent_in_eb(eb, bytenr,
>>                                                       *extent_item_pos, &eie);
>> +                             if (ret < 0)
>> +                                     goto out;
>>                               ref->inode_list = eie;
>>                               free_extent_buffer(eb);
>>                       }
>
> The eb needs to be freed before the goto.
>
> And that there's now *three* eb freeing paths in this block should be a
> hint that it should be refactored into a function that frees the eb
> before returning.

Thanks, missed that previously.

>
> - z
diff mbox

Patch

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 8bc5e8c..1ba87c5 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -935,6 +935,8 @@  again:
 				}
 				ret = find_extent_in_eb(eb, bytenr,
 							*extent_item_pos, &eie);
+				if (ret < 0)
+					goto out;
 				ref->inode_list = eie;
 				free_extent_buffer(eb);
 			}