Re: [libvirt] [PATCH 03/15] Introduce a generic object for using network sockets
On Thu, Dec 23, 2010 at 11:53:41AM +0800, Wen Congyang wrote:
At 12/16/2010 07:21 PM, Daniel P. Berrange Write:
> Introduces a simple wrapper around the raw POSIX sockets APIs
> and name resolution APIs. Allows for easy creation of client
> and server sockets with correct usage of name resolution APIs
> for protocol agnostic socket setup.
>
> It can listen for UNIX and TCP stream sockets.
>
> It can connect to UNIX, TCP streams directly, or indirectly
> to UNIX sockets via an SSH tunnel or external command
>
> * src/Makefile.am: Add to libvirt-net-rpc.la
> * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Generic
> sockets APIs
> ---
> .x-sc_avoid_write | 1 +
> po/POTFILES.in | 1 +
> src/Makefile.am | 3 +-
> src/rpc/virnetsocket.c | 723 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/rpc/virnetsocket.h | 100 +++++++
> 5 files changed, 827 insertions(+), 1 deletions(-)
> create mode 100644 src/rpc/virnetsocket.c
> create mode 100644 src/rpc/virnetsocket.h
>
<snip>
> +static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr,
> + virSocketAddrPtr remoteAddr,
> + int fd, int errfd, pid_t pid)
> +{
> + virNetSocketPtr sock;
> + int no_slow_start = 1;
> +
> + VIR_DEBUG("localAddr=%p remoteAddr=%p fd=%d errfd=%d pid=%d",
> + localAddr, remoteAddr,
> + fd, errfd, pid);
> +
> + if (virSetCloseExec(fd) < 0 ||
> + virSetNonBlock(fd) < 0)
> + return NULL;
> +
> + if (VIR_ALLOC(sock) < 0) {
> + virReportOOMError();
> + return NULL;
> + }
> +
> + sock->localAddr = *localAddr;
In the function virNetSocketNewConnectCommand():
if (!(*retsock = virNetSocketNew(NULL, NULL, sv[0], errfd[0], pid)))
goto error;
So we should check whether localAddr is NULL before dereferencing it.
Yep, I've just noticed that too.
> + if (remoteAddr)
> + sock->remoteAddr = *remoteAddr;
> + sock->fd = fd;
> + sock->errfd = errfd;
> + sock->pid = pid;
> +
> + /* Disable nagle */
> + if (sock->localAddr.data.sa.sa_family != AF_UNIX)
> + setsockopt (fd, IPPROTO_TCP, TCP_NODELAY,
> + (void *)&no_slow_start,
> + sizeof(no_slow_start));
> +
> +
> + DEBUG("local family = %d remote family = %d",
> + localAddr->data.sa.sa_family,
> + remoteAddr ? remoteAddr->data.sa.sa_family : 0);
we should check localAddr
> + if (!(sock->localAddrStr = virSocketFormatAddrFull(localAddr, true,
";")))
> + goto error;
we should check localAddr
> +
> + if (remoteAddr &&
> + !(sock->remoteAddrStr = virSocketFormatAddrFull(remoteAddr, true,
";")))
> + goto error;
> +
> + sock->client = localAddr && !remoteAddr ? false : true;
Instead of this bogus logic, I'm going to pass an
expliciti 'bool isClient' param into this method.
> +
> + VIR_DEBUG("sock=%p localAddrStr=%s remoteAddrStr=%s",
> + sock, NULLSTR(sock->localAddrStr),
NULLSTR(sock->remoteAddrStr));
> +
> + return sock;
> +
> +error:
> + virNetSocketFree(sock);
> + return NULL;
> +}
> +
> +
<snip>
> + virCommandSetInputFD(cmd, sv[1]);
> + virCommandSetOutputFD(cmd, &sv[1]);
> + virCommandSetOutputFD(cmd, &errfd[1]);
s/virCommandSetOutputFD/virCommandSetErrorFD/
Thanks,
Daniel