[Ocfs2-tools-devel] [PATCH 1/1] ocfs2-tools: fix wrong pointer to pointer in mount.ocfs2
Tiger Yang
tiger.yang at oracle.com
Thu Mar 31 18:42:13 PDT 2011
On 04/01/2011 12:59 AM, Sunil Mushran wrote:
> On 03/30/2011 07:12 PM, Tiger Yang wrote:
>> 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.
>
> Fair enough.
>
>>>
>>> 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.
>
> xstrconcat3() also allocates a new buffer.
>
> This problem is new. And this code is fairly old. What changed?
the original code in if statement is mo.xtra_opts, but commit
52bae5e7a358e927a1e841ead2c6a95cf68c5db1 use &mo.xtra_opts in
ret = add_mount_options(fs, &cluster, &mo.xtra_opts);
Thanks,
Tiger
More information about the Ocfs2-tools-devel
mailing list