[libvirt] [PATCH 03/15] Introduce a generic object for using network sockets

[Daniel P. Berrange]

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