Hello Dev Team,

I would like to understand more how we treat Coverity resource leak issues,
especially the one below.

packet-nfs.c
line 4891
CID 1682039: (#2 of 2): Resource leak (RESOURCE_LEAK)
55. leaked_storage: Variable text_start going out of scope leaks the
storage it points to.

https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-nfs.c?ref_type=heads#L4887

if (!(rmask & accvs[itype].value) && access_subitem) {
    const char* *text_start* = proto_item_get_text(pinfo->pool,
access_subitem);
    const char* text_end = text_start ? strrchr(text_start, ':') : NULL;
    if (text_start && text_end)
        proto_item_set_text(access_subitem, "%.*s: %s", (int)(text_end -
text_start), text_start, mtype == 'C' ? "not asked" : "unknown");
} else if (mtype == 'C' && access_subitem) {
    proto_item_append_text(access_subitem, "?" );
}

The return value of proto_item_get_text() is allocated with
wmem_strdup(pinfo->pool, ...).
So it means it will be freed properly eventually since the allocator is not
NULL.

But do we still need to consider the Coverity resource leak and add
wmem_free(pinfo->pool,text_start); after proto_item_set_text?
Or this one one we simply ignore?

if (!(rmask & accvs[itype].value) && access_subitem) {
    const char* text_start = proto_item_get_text(pinfo->pool,
access_subitem);
    const char* text_end = text_start ? strrchr(text_start, ':') : NULL;
    if (text_start && text_end)
        proto_item_set_text(access_subitem, "%.*s: %s", (int)(text_end -
text_start), text_start, mtype == 'C' ? "not asked" : "unknown");
        *wmem_free(pinfo->pool,text_start);*
} else if (mtype == 'C' && access_subitem) {
    proto_item_append_text(access_subitem, "?" );
}

Thank you.

Regards,
Tamas
_______________________________________________
Wireshark-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to