[Ocfs2-tools-devel] [PATCH 1/1] ocfs2-tools: fix wrong pointer to pointer in mount.ocfs2

[Tiger Yang]

On 03/31/2011 08:16 AM, Sunil Mushran wrote:
> On 03/29/2011 08:26 PM, Tiger Yang wrote:
>> commit 52bae5e7a358e927a1e841ead2c6a95cf68c5db1 use the wrong
>> pointer to pointer in if statement. This patch fixes this problem
>> and clean the codes to create option string.
>>
>> Signed-off-by: Tiger Yang<tiger.yang at oracle.com>
>> ---
>>   mount.ocfs2/mount.ocfs2.c |    8 +++-----
>>   1 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/mount.ocfs2/mount.ocfs2.c b/mount.ocfs2/mount.ocfs2.c
>> index a5e117d..4925f20 100644
>> --- a/mount.ocfs2/mount.ocfs2.c
>> +++ b/mount.ocfs2/mount.ocfs2.c
>> @@ -133,11 +133,9 @@ static errcode_t add_mount_options(ocfs2_filesys 
>> *fs,
>>       add = OCFS2_HB_LOCAL;
>>
>>   addit:
>> -    if (optstr&&  *optstr) {
>> -        extra = xstrndup(*optstr, strlen(*optstr) + strlen(add) + 1);
>> -        if (extra)
>> -            extra = xstrconcat3(extra, ",", add);
>> -    } else
>> +    if (*optstr&&  *(*optstr))
>> +        extra = xstrconcat3(*optstr, ",", add);
>
> This function calls free(*optstr). Probably not what we want as
> it could lead to a double free.
  I know it will free the old optstr, and that is I want. Because the 
next line is
  *optstr = extra;  then in main(), will free the extra.
         if (mo.xtra_opts)
                 free(mo.xtra_opts);
But the old optstr will never get free, will cause memory leak.
>
> This code has been there forever. Why is this problem happening now?
I read the codes carefully, I can not understand why we copy string 
length is (strlen(*optstr) + strlen(add) + 1) from optstr,
and then free that extra in xstrconcat3(extra, ",", add).
Actually, these three lines do not raise the bug 11929515, it only cause 
memory leak.
The wrong use of pointer to pointer in if statement will cause the 
condition true and then add a comma in option string.

Thanks,
tiger