* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On 19 March 2014 11:13, Dr. David Alan Gilbert (git)
> <dgilb...@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> >
> > Coverity defects 1005733 & 1005734 complain about passing a -ve value
> > to closesocket in the error paths on incoming migration.
> >
> > Stash the error value and print it in the message (previously we gave
> > no indication of the reason for the failure)
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > ---
> > migration-tcp.c | 11 ++++++-----
> > migration-unix.c | 11 ++++++-----
> > 2 files changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/migration-tcp.c b/migration-tcp.c
> > index 782572d..5c96cd3 100644
> > --- a/migration-tcp.c
> > +++ b/migration-tcp.c
> > @@ -56,19 +56,20 @@ static void tcp_accept_incoming_migration(void *opaque)
> > socklen_t addrlen = sizeof(addr);
> > int s = (intptr_t)opaque;
> > QEMUFile *f;
> > - int c;
> > + int c, err;
> >
> > do {
> > c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
> > - } while (c == -1 && socket_error() == EINTR);
> > + err = socket_error();
> > + } while (c == -1 && err == EINTR);
> > qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
> > closesocket(s);
> >
> > DPRINTF("accepted migration\n");
> >
> > - if (c == -1) {
> > - fprintf(stderr, "could not accept migration connection\n");
> > - goto out;
> > + if (c < 0) {
>
> Why change the condition? Or alternatively, why use <0 here
> but retain == -1 in the while condition above?
Because according to the manpage of accept(2) it's defined to return
-1 on error, or a +ve fd if it works, that while loop is purely checking
for the well defined case of EINTR i.e. -1/errno=EINTR; so the -1 in
the while loop is specific to the defined error case; I'm using < 0
here to catch -1 (which is what should happen) and anything undefined -
and thus make sure the close has a valid value.
>
> > + fprintf(stderr, "could not accept migration connection (%d)\n",
> > err);
>
> Bit unfriendly not to convert the errno to a string.
I could reroll it with a strerror.
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
