Re: [Qemu-devel] [Qemu-trivial] [RESEND PATCH 1/3] socket: remove redundant check

[Peter Maydell <peter.maydell@linaro.org>]

On 1 August 2012 11:50, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> This isn't obvious.  It looks like the intent of the if (!e->ai_next) is
> to suppress the error so that the next iteration of the *outer* loop can
> succeed.

Yeah, we only call it an error on the last time round. This
loop is a bit confusingly structured, since we're effectively
handling the failure in several places at once: we always
fprintf() something, then we set the error on the last time
round the loop, then at the end of the loop we fprintf again.
We also duplicate the loop termination condition.

It might be better to have an Error *local_err in this function.
Then we could unconditionally call error_set() for any failures,
passing &local_err. Then at the end of the loop we can call
error_propagate(errp, local_err) to pass an error up if we
didn't succeed at all. Unfortunately you'd have to do
    if (error_is_set(&local_err)) {
        error_free(&local_err);
    }
    error_set(&local_err, QERR_whatever);
for the error setting, since error_set() will assert if you try
to set an error twice.

[Another demonstration of the fprintf errors being much more
useful than the error_set mechanisms at the moment, incidentally.
We can get away with the fprintfs because the only caller of this
function which passes in a non-NULL errp is the migration code
called from vl.c, which is just going to fprintf and exit on
error anyway.]

-- PMM