system/qemu
system/qemu
1
0
Fork 0
mirror of https://gitlab.com/qemu-project/qemu synced 2024-09-20 03:01:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

hw/xen: Clarify (lack of) error handling in transaction_commit()

Browse source 
Coverity was unhappy (CID 1508359) because we didn't check the return of
init_walk_op() in transaction_commit(), despite doing so at every other
call site.

Strictly speaking, this is a false positive since it can never fail. It
only fails for invalid user input (transaction ID or path), and both of
those are hard-coded to known sane values in this invocation.

But Coverity doesn't know that, and neither does the casual reader of the
code.

Returning an error here would be weird, since the transaction *is*
committed by this point; all the walk_op is doing is firing watches on
the newly-committed changed nodes. So make it a g_assert(!ret), since
it really should never happen.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Reviewed-by: Paul Durrant <paul@xen.org>
Message-Id: <20076888f6bdf06a65aafc5cf954260965d45b97.camel@infradead.org>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
This commit is contained in:
David Woodhouse 2023-06-20 18:58:55 +01:00 committed by Anthony PERARD
parent 802341823f
commit ace33a0e5a
1 changed files with 11 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
hw/i386/kvm/xenstore_impl.c
View file

@ -1022,6 +1022,7 @@ static int transaction_commit(XenstoreImplState *s, XsTransaction *tx)
{
struct walk_op op;
XsNode **n;
int ret;
if (s->root_tx != tx->base_tx) {
return EAGAIN;
@ -1032,7 +1033,16 @@ static int transaction_commit(XenstoreImplState *s, XsTransaction *tx)
s->root_tx = tx->tx_id;
s->nr_nodes = tx->nr_nodes;
init_walk_op(s, &op, XBT_NULL, tx->dom_id, "/", &n);
ret = init_walk_op(s, &op, XBT_NULL, tx->dom_id, "/", &n);
/*
* There are two reasons why init_walk_op() may fail: an invalid tx_id,
* or an invalid path. We pass XBT_NULL and "/", and it cannot fail.
* If it does, the world is broken. And returning 'ret' would be weird
* because the transaction *was* committed, and all this tree walk is
* trying to do is fire the resulting watches on newly-committed nodes.
*/
g_assert(!ret);
op.deleted_in_tx = false;
op.mutating = true;