Hi Eric,
None of this is important, and may not even be bugs, but here are a
few issues raised with the latest run of Coverity on nbdkit 1.30.7.
**********************************************************************
*** filters/multi-conn
It seems as if "group_vector" is really leaked from this filter. I
think it needs an unload handler to free it.
Error: GCC_ANALYZER_WARNING (CWE-401): [#def75]
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:195:14:
warning[-Wanalyzer-malloc-leak]: leak of 'g'
nbdkit-1.30.7/common/utils/vector.h:44: included_from: Included from here.
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:46: included_from: Included from
here.
nbdkit-1.30.7/common/utils/cleanup.h:56:3: note: in expansion of macro
'ACQUIRE_LOCK_FOR_CURRENT_SCOPE_1'
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:208:3: note: in expansion of macro
'ACQUIRE_LOCK_FOR_CURRENT_SCOPE'
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:46: included_from: Included from
here.
nbdkit-1.30.7/common/utils/vector.h:132:12: note: in definition of macro
'DEFINE_VECTOR_TYPE'
nbdkit-1.30.7/common/utils/vector.h:132:12: note: in definition of macro
'DEFINE_VECTOR_TYPE'
nbdkit-1.30.7/common/utils/vector.h:116:3: note: in definition of macro
'DEFINE_VECTOR_TYPE'
nbdkit-1.30.7/common/utils/vector.h:44: included_from: Included from here.
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:46: included_from: Included from
here.
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:87:1: note: in expansion of macro
'DEFINE_VECTOR_TYPE'
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:46: included_from: Included from
here.
nbdkit-1.30.7/common/utils/vector.h:132:12: note: in definition of macro
'DEFINE_VECTOR_TYPE'
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:45: included_from: Included from
here.
nbdkit-1.30.7/common/utils/cleanup.h:58:41: note: in definition of macro
'ACQUIRE_LOCK_FOR_CURRENT_SCOPE_1'
nbdkit-1.30.7/common/include/unique-name.h:40:37: note: in expansion of macro
'XXUNIQUE_NAME'
nbdkit-1.30.7/common/include/unique-name.h:41:34: note: in expansion of macro
'XUNIQUE_NAME'
nbdkit-1.30.7/common/utils/cleanup.h:56:45: note: in expansion of macro
'NBDKIT_UNIQUE_NAME'
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:208:3: note: in expansion of macro
'ACQUIRE_LOCK_FOR_CURRENT_SCOPE'
# 193| r = next->can_multi_conn (next);
# 194| if (r == -1)
# 195|-> return -1;
# 196| if (r == 0)
# 197| h->mode = EMULATE;
Error: CLANG_WARNING: [#def76]
nbdkit-1.30.7/filters/multi-conn/multi-conn.c:227:15: warning[unix.Malloc]:
Potential leak of memory pointed to by 'g'
# 225| }
# 226| if (group_vector_append (&groups, g) == -1)
# 227|-> return -1;
# 228| g->name = h->name;
# 229| h->name = NULL;
**********************************************************************
*** server/protocol-handshake-newstyle.c
I think Coverity has a point here. Certainly there is code before
this point which assigns conn->top_context = NULL, and it's not
immediately clear to me that the code that Coverity has highlighted is
unreachable in that case.
Error: CLANG_WARNING: [#def234]
nbdkit-1.30.7/server/protocol-handshake-newstyle.c:657:34:
warning[core.NonNullParamChecker]: Null pointer passed to 1st parameter
expecting 'nonnull'
# 655| case NBD_INFO_DESCRIPTION:
# 656| {
# 657|-> const char *desc = backend_export_description
(conn->top_context);
# 658|
# 659| if (!desc) {
Error: CLANG_WARNING: [#def235]
nbdkit-1.30.7/server/protocol-handshake-newstyle.c:676:19:
warning[core.NonNullParamChecker]: Null pointer passed to 1st parameter
expecting 'nonnull'
# 674| uint32_t minimum, preferred, maximum;
# 675|
# 676|-> if (backend_block_size (conn->top_context,
# 677| &minimum, &preferred,
&maximum) == -1)
# 678| return -1;
**********************************************************************
*** server/public.c
Any clue about what this error means?
Error: ASSERT_SIDE_EFFECT (CWE-1006): [#def239]
nbdkit-1.30.7/server/public.c:728: assert_side_effect: Argument "quit"
of assert() has a side effect because the variable is volatile. The containing
function might work differently in a non-debug build.
# 726| * event, we know the connection should be shutting down.
# 727| */
# 728|-> assert (quit ||
# 729| (conn && conn->nworkers > 0 &&
connection_get_status () < 1) ||
# 730| (conn && (fds[2].revents & (POLLRDHUP | POLLHUP
| POLLERR |
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
On Tue, Jul 12, 2022 at 07:22:31PM +0100, Richard W.M. Jones wrote:> Hi Eric, > > None of this is important, and may not even be bugs, but here are a > few issues raised with the latest run of Coverity on nbdkit 1.30.7. > > > ********************************************************************** > *** filters/multi-conn > > It seems as if "group_vector" is really leaked from this filter. I > think it needs an unload handler to free it. > > > Error: GCC_ANALYZER_WARNING (CWE-401): [#def75] > nbdkit-1.30.7/filters/multi-conn/multi-conn.c:195:14: warning[-Wanalyzer-malloc-leak]: leak of 'g'> nbdkit-1.30.7/common/utils/vector.h:44: included_from: Included from here. > # 193| r = next->can_multi_conn (next); > # 194| if (r == -1) > # 195|-> return -1; > # 196| if (r == 0) > # 197| h->mode = EMULATE; > > Error: CLANG_WARNING: [#def76] > nbdkit-1.30.7/filters/multi-conn/multi-conn.c:227:15: warning[unix.Malloc]: Potential leak of memory pointed to by 'g'Yep, seems like an easy fix to make.> ********************************************************************** > *** server/protocol-handshake-newstyle.c > > I think Coverity has a point here. Certainly there is code before > this point which assigns conn->top_context = NULL, and it's not > immediately clear to me that the code that Coverity has highlighted is > unreachable in that case. > > > Error: CLANG_WARNING: [#def234] > nbdkit-1.30.7/server/protocol-handshake-newstyle.c:657:34: warning[core.NonNullParamChecker]: Null pointer passed to 1st parameter expecting 'nonnull' > # 655| case NBD_INFO_DESCRIPTION: > # 656| { > # 657|-> const char *desc = backend_export_description (conn->top_context); > # 658| > # 659| if (!desc) { > > Error: CLANG_WARNING: [#def235] > nbdkit-1.30.7/server/protocol-handshake-newstyle.c:676:19: warning[core.NonNullParamChecker]: Null pointer passed to 1st parameter expecting 'nonnull' > # 674| uint32_t minimum, preferred, maximum; > # 675| > # 676|-> if (backend_block_size (conn->top_context, > # 677| &minimum, &preferred, &maximum) == -1) > # 678| return -1;I'll look into those.> > > ********************************************************************** > *** server/public.c > > Any clue about what this error means? > > > Error: ASSERT_SIDE_EFFECT (CWE-1006): [#def239] > nbdkit-1.30.7/server/public.c:728: assert_side_effect: Argument "quit" of assert() has a side effect because the variable is volatile. The containing function might work differently in a non-debug build. > # 726| * event, we know the connection should be shutting down. > # 727| */ > # 728|-> assert (quit || > # 729| (conn && conn->nworkers > 0 && connection_get_status () < 1) || > # 730| (conn && (fds[2].revents & (POLLRDHUP | POLLHUP | POLLERR |Interesting. Because 'quit' is volatile, Coverity is arguing that the mere act of reading it may trigger a side effect; where an NDEBUG build skips the read. A (maybe non-obvious) fix would be: bool has_quit = quit; assert (has_quit || ... ); so that the contents of the assert() no longer read a volatile variable. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org