Re: [Qemu-devel] [PATCH v2 00/16] net: Move legacy QEMU VLAN code into net/hub.c

[Laszlo Ersek <lersek@redhat.com>]

I'd like to review v2 tomorrow (ahem, today), ie. not right now, but
this catches my eye:

On 07/24/12 17:35, Stefan Hajnoczi wrote:

>  * Drop spurious closesocket(fd), probably merge conflict [Stefan]

I saw it last time and I thought you had fixed a leak with it on the
side. The function looks as follows (right after applying [01 .. 02]
from v1):

[net/socket.c]

  376  static void net_socket_accept(void *opaque)
  377  {
  378      NetSocketListenState *s = opaque;
  379      NetSocketState *s1;
  380      struct sockaddr_in saddr;
  381      socklen_t len;
  382      int fd;
  383
  384      for(;;) {
  385          len = sizeof(saddr);
  386          fd = qemu_accept(s->fd, (struct sockaddr *)&saddr, &len);
  387          if (fd < 0 && errno != EINTR) {
  388              return;
  389          } else if (fd >= 0) {
  390              break;
  391          }
  392      }
  393      s1 = net_socket_fd_init(s->peer, s->model, s->name, fd, 1);
  394      if (!s1) {
  395          closesocket(fd);
  396      } else {
  397          snprintf(s1->nc.info_str, sizeof(s1->nc.info_str),
  398                   "socket: connection from %s:%d",
  399                   inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
  400      }
  401  }

If net_socket_fd_init() fails, then the file descriptor extracted by
qemu_accept() should be closed. The fix is not strongly related to the
series but I figured I'd let it slide :)

... Oh. net_socket_fd_init() takes ownership even if it fails, in which
case it calls closesocket() itself.

Such functions need a banner :)

Laszlo