On Thu, 22 May 2025 at 11:33, Daniel P. Berrangé <[email protected]> wrote:
>
> From: Juraj Marcin <[email protected]>
>
> To get a listening socket, we need to first create a socket, try binding
> it to a certain port, and lastly starting listening to it. Each of these
> operations can fail due to various reasons, one of them being that the
> requested address/port is already in use. In such case, the function
> tries the same process with a new port number.
>
> This patch refactors the port number loop, so the success path is no
> longer buried inside the 'if' statements in the middle of the loop. Now,
> the success path is not nested and ends at the end of the iteration
> after successful socket creation, binding, and listening. In case any of
> the operations fails, it either continues to the next iteration (and the
> next port) or jumps out of the loop to handle the error and exits the
> function.
>
> Signed-off-by: Juraj Marcin <[email protected]>
> Reviewed-by: Daniel P. Berrangé <[email protected]>
> Signed-off-by: Daniel P. Berrangé <[email protected]>
> ---
> util/qemu-sockets.c | 51 ++++++++++++++++++++++++---------------------
> 1 file changed, 27 insertions(+), 24 deletions(-)
>
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 4a878e0527..329fdbfd97 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
Hi; Coverity complains about this code (CID 1610306):
> @@ -303,11 +303,20 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
> port_min = inet_getport(e);
> port_max = saddr->has_to ? saddr->to + port_offset : port_min;
> for (p = port_min; p <= port_max; p++) {
> + if (slisten >= 0) {
> + /*
> + * We have a socket we tried with the previous port. It
> cannot
> + * be rebound, we need to close it and create a new one.
> + */
> + close(slisten);
> + slisten = -1;
Here we set slisten to -1 ...
> + }
> inet_setport(e, p);
...but then two lines later we unconditionally set slisten to
something else, so the -1 assignment is overwritten without being
used.
> slisten = create_fast_reuse_socket(e);
What was the intention here ?
thanks
-- PMM
