[Ocfs2-tools-devel] [PATCH] o2image: 'blk' and 'ofs->ost' need to be freed when error happens in ocfs2_image_load_bitmap()

[piaojun]

Hi Joseph,

On 2018/4/3 16:47, Joseph Qi wrote:
> 
> 
> On 18/4/3 15:49, piaojun wrote:
>> 1. 'blk' should be freed if ocfs2_image_alloc_bitmap() failed.
>> 2. 'ofs->ost' should be freed when ocfs2_malloc_block() failed.
>>
>> Signed-off-by: Jun Piao <piaojun at huawei.com>
>> ---
>>  libocfs2/image.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/libocfs2/image.c b/libocfs2/image.c
>> index 4ceab67..fb72bad 100644
>> --- a/libocfs2/image.c
>> +++ b/libocfs2/image.c
>> @@ -175,7 +175,7 @@ errcode_t ocfs2_image_load_bitmap(ocfs2_filesys *ofs)
>>  	ost = ofs->ost;
>>  	ret = ocfs2_malloc_block(ofs->fs_io, &blk);
>>  	if (ret)
>> -		return ret;
>> +		goto out;
>>
>>  	/* read ocfs2 image header */
>>  	ret = io_read_block(ofs->fs_io, 0, 1, blk);
>> @@ -204,7 +204,7 @@ errcode_t ocfs2_image_load_bitmap(ocfs2_filesys *ofs)
>>
>>  	ret = ocfs2_image_alloc_bitmap(ofs);
>>  	if (ret)
>> -		return ret;
>> +		goto out;
>>
>>  	/* load bitmap blocks ocfs2 image state */
>>  	bits_set = 0;
>> @@ -233,8 +233,9 @@ errcode_t ocfs2_image_load_bitmap(ocfs2_filesys *ofs)
>>  	}
>>
>>  out:
>> -	if (blk)
>> -		ocfs2_free(&blk);
>> +	ocfs2_free(&blk);
>> +	if (ret)
>> +		ocfs2_free(&ofs->ost);
>>  	return ret;
>>  }
>>
> I'm afraid there are 2 problems here.
> 1) blk may be not initialized, so we have to initialize it to NULL and it is better
> to check first when free.
Good catch, I will fix this problem in patch v2.

> 2) bitmap not be freed when read fails.
Yes, ocfs2_image_free_bitmap() need to be called before freeing 'ofs->ost'.

thanks,
Jun
> 
> Thanks,
> Joseph 
>