[lustre-devel] Isn't there a memory leak here !!!

[NeilBrown]

On Wed, Jan 20 2021, Sudheendra Sampath wrote:

> Hi,
>
> While doing transaction code analysis, I found the following in
> lustre/target/tgt_lastrcvd.c.
>
>     552 int tgt_new_client_cb_add(struct thandle *th, struct obd_export
> *exp)
>     553 {
>     554         struct tgt_new_client_callback  *ccb;
>     555         struct dt_txn_commit_cb         *dcb;
>     556         int                              rc;
>     557
> *    558         OBD_ALLOC_PTR(ccb);*
>     559         if (ccb == NULL)
>     560                 return -ENOMEM;
>     561
>     562         ccb->lncc_exp = class_export_cb_get(exp);
>     563
>     564         dcb = &ccb->lncc_cb;
>     565         dcb->dcb_func = tgt_cb_new_client;
>     566         INIT_LIST_HEAD(&dcb->dcb_linkage);
>     567         strlcpy(dcb->dcb_name, "tgt_cb_new_client",
> sizeof(dcb->dcb_name));
>     568
>     569         rc = dt_trans_cb_add(th, dcb);
>     570         if (rc) {
>     571                 class_export_cb_put(exp);
> *    572                 OBD_FREE_PTR(ccb);*
>     573         }
>     574         return rc;
>     575 }
>
> OBD_FREE_PTR() is in the condition block which means that the expectation
> is dt_trans_cb_add() returns something "!= 0".
>
> From the code, osd_trans_cb_add() and osp_trans_cb_add() returns zero
> value.  So, my point is OBD_FREE_PTR() should be outside the condition
> block.
>
> Please correct me if my understanding is incorrect.

If dt_trans_cb_add() returns zero, then 'dcb' has been added to some
list of transactions.  dcb is a pointer to a 'struct dt_txn_commit_cb'
structure embedded inside 'ccb'.

So when rc==0, the memory allocated for ccb is now attached to a list of
transactions, so it would be wrong to free it.  If the attachment
(dt_trans_cb_add()) failed, then it needs to be freed.

So I think the code is correct as it stands.

Thanks,
NeilBrown


>
> -- 
> Regards
>
> Sudheendra Sampath
> _______________________________________________
> lustre-devel mailing list
> lustre-devel at lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 853 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20210121/c1fc6c5d/attachment.sig>