On 2020/10/17 3:51, Martin Wilck wrote:
> On Fri, 2020-10-16 at 14:23 +0800, lixiaokeng wrote:
>> When multipath -F are executed firstly and multipath -v2 or
>> -d are executed later, asan will warn memory leaks. The
>> reason is that the mpp allocated in coalesce_paths isn't
>> freed. Here we add newmp in configure(multipath) to store
>> mpp and free it.
>>
>> Signed-off-by: Lixiaokeng <lixiaokeng@xxxxxxxxxx>
>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@xxxxxxxxxx>
>> Signed-off-by: Linfeilong <linfeilong@xxxxxxxxxx>
>
> Besides what Ben noted already, I think you shouldn't force callers to
> pass a non-NULL "newmp". The tricky part is to make sure that paths
> aren't handled repeatedly in the CMD_DRY_RUN case. Currently pp->mpp !=
> NULL is the condition used by coalesce_paths() to check if a path has
> already been dealt with; if you simply call remove_map(), that won't
> work any more. I suppose you realized that, and that's why you
> introduced the non-NULL newmp in multipath (you should have mentioned
> that in the changelog message).
>
> I suggest to have callers pass a "vector *pnewmp" instead of "vector
> newmp", always allocate newmp in coalesce_paths(), and upon return,
> either free newmp, or assign it to the pointer passed by the caller:
>
> if (pnewmp)
> *pnewmp = newmp;
> else
> free_multipathvec(newmp, KEEP_PATHS);
>
> Regards,
> Martin
>
Hi Martin,
Thanks for your review. It is a great idea with your suggestion.
I think it's better that callers pass a "vector mpvec" instead of
"vector newmp", either copy mpvec to newmp or allocate newmp at the
beginning of coalesce_paths, and free newmp or not at the end:
int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
int force_reload, enum mpath_cmds cmd)
......
if (mpvec)
newmp = mpvec;
else
newmp = vector_alloc();
if (!newmp) {
condlog(0, "can not allocate newmp");
return ret;
}
......
if (!mpvec)
free_multipathvec(newmp, KEEP_PATHS);
About conflict, can you give me the url of the code with your
patchset? If I just change coalease_paths, will it have some
conflicts?
Regards,
Lixiaokeng
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
