Hi,
Coverity emits warnings in a number of places about code like this
errout:
if (zhp != NULL)
zfs_close(zhp);
One example warning is
CID 166345 (#1 of 1): Dereference before null check
(REVERSE_INULL)check_after_deref: Null-checking
zhp suggests that it may be null, but it has already been dereferenced on all
paths leading to the
check
A previous PR was merged that changed the code to look like this
The warning is that
errout:
ASSERT3P(zhp, !=, NULL);
zfs_close(zhp);
The question is; is this just changing the code to satisfy coverity? If so,
that seems like a bad
idea.
Would ZFS be open to moving the null check into zfs_close()? (extra white space
for clarity).
/*
* Release a ZFS handle. Nothing to do but free the associated memory.
*/
void
zfs_close(zfs_handle_t *zhp)
{
+ if (zhp == NULL)
+ return;
if (zhp->zfs_mntopts)
free(zhp->zfs_mntopts);
nvlist_free(zhp->zfs_props);
nvlist_free(zhp->zfs_user_props);
nvlist_free(zhp->zfs_recvd_props);
free(zhp);
}
Does this improve the code?
Does it quiet coverity?
Long email for such a trivial question ;)
Last question; is this the correct channel to open general questions relating
to the code base or do
you guys prefer issues on GitHub?
thanks,
Tobin.
------------------------------------------
openzfs-developer
Archives:
https://openzfs.topicbox.com/groups/developer/discussions/T9490ee2cf4250913-M056337cdff4c0201d3cf03e0
Powered by Topicbox: https://topicbox.com
