[libvirt] [PATCH 06/10] Add a generic internal API for handling any FD based stream

[Daniel P. Berrange]

On Tue, Nov 02, 2010 at 10:30:54AM -0600, Eric Blake wrote:
> On 11/02/2010 05:42 AM, Daniel P. Berrange wrote:
> >>
> >>> +#include <netinet/in.h>
> >>> +#include <netinet/tcp.h>
> >>
> >> Likewise for HAVE_NETINET_TCP_H.
> > 
> > Doesn't gnulib take care of TCP socket portability for us ?
> 
> So far, gnulib provides <netinet/in.h>, but not <netinet/tcp.h>.  But
> what exactly are we using that's only in netinet/tcp.h?  It may be easy
> to port to gnulib, since mingw does have tcp support (sys/un.h is the
> much harder task, since mingw has no notion of unix sockets).

It seems netinet/tcp.h is not required on either platform and we
don't need sys/un.h on Win32.

> >>> +static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes)
> >>
> >> Should this return ssize_t...
> > 
> > No, this has to return an int, to match the public API calling
> > convention.
> 
> Then we have to check for overflow, and explicitly return an error if
> nbytes > INT_MAX.

Ok

> >>> +    if ((st->flags & VIR_STREAM_NONBLOCK) &&
> >>> +        (!S_ISCHR(sb.st_mode) &&
> >>> +         !S_ISFIFO(sb.st_mode))) {
> >>
> >> Should we also permit S_ISSOCK as an fd that supports reliable
> >> non-blocking behavior?
> > 
> > AFAIK, there's no way to open a socket as a path on the
> > filesystem is there ?  So there'd be no way that open(path)
> > could return an FD for which S_ISSOCK() is true.
> 
> You're probably right that it's not possible to bind a socket to a
> standard file system.  But it is possible via procfs (think
> /proc/nnn/fd/nnn of a process which already has a socket open), and
> there might also be some magic /dev/ or /sys/ mappings that can provide
> a socket (at any rate, bash provides magic handling of
> /dev/tcp/host/port, whether or not the kernel also has a magic
> filesystem to provide that support automatically).  So it boils down to
> a question of whether we want to permit or reject sockets, on the
> pre-condition that the user finds a way to hand us a filename that
> resolves to a socket.

I tried add S_ISSOCK() too, but gnulib #defines this to '0' which
then causes gcc to issue a warning that the expression always
evaluates to true, because we build with -Wlogical-op. So I've
left this change out.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|