09.09.2019 20:32, Peter Maydell wrote: > On Thu, 15 Aug 2019 at 19:34, Eric Blake <ebl...@redhat.com> wrote: >> >> From: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> >> It's needed to provide keepalive for nbd client to track server >> availability. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> Message-Id: <20190725094937.32454-1-vsement...@virtuozzo.com> >> Reviewed-by: Markus Armbruster <arm...@redhat.com> >> Acked-by: Daniel P. Berrangé <berra...@redhat.com> >> [eblake: Fix error message typo] >> Signed-off-by: Eric Blake <ebl...@redhat.com> > > Hi; Coverity spots a bug in this change (CID 1405300):
Two coverity bugs on my patches, I'm sorry :( How can I run this coverity locally? > >> @@ -458,6 +464,19 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error >> **errp) >> } > > At this point in the function, we might be in one of > two cases: > (1) sock >= 0 : the connect succeeded > (2) sock < 0 : connect failed, we have just called > error_propagate() and are using the same end-of-function > codepath to free 'res' before returning the error code. > >> >> freeaddrinfo(res); >> + >> + if (saddr->keep_alive) { >> + int val = 1; >> + int ret = qemu_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, >> + &val, sizeof(val)); > > ...but here we use 'sock' assuming it is valid. > >> + >> + if (ret < 0) { >> + error_setg_errno(errp, errno, "Unable to set KEEPALIVE"); >> + close(sock); >> + return -1; >> + } >> + } >> + >> return sock; >> } > > If we want to add more "actual work" at this point in > the function we should probably separate out the failed-case > codepath, eg by changing > > > if (sock < 0) { > error_propagate(errp, local_err); > } > > freeaddrinfo(res); > > into > > freeaddrinfo(res); > > if (sock < 0) { > error_propagate(errp, local_err); > return sock; > } > > > > thanks > -- PMM > Thanks, this should work, I'll send a patch soon. -- Best regards, Vladimir