On 11/02/2010 05:42 AM, Daniel P. Berrange wrote: >>> - VIR_FROM_AUDIT /* Error from auditing subsystem */ >>> + VIR_FROM_AUDIT, /* Error from auditing subsystem */ >>> + VIR_FROM_STREAMS, /* Error from I/O streams */ >>> } virErrorDomain; >> >> Is the switch from C89 style (no trailing comma) to the C99 style >> (optional trailing comma permitted) intentional? Personally, I like it, >> since adding a new value at the end no longer requires a random-looking >> diff of the previous entry just to add a comma. > > IMHO, leaving off the comma needlessly enlarges future patches > because 2 lines have to be changed to add an entry instead > of just one. Sounds like we're in violent agreement here, about favoring C99 syntax. Is it worth a generic cleanup patch for other enum declarations that don't yet use trailing commas, or should we just save it for an as-encountered basis? >>> +#include <sys/un.h> >> >> but this is missing on Mingw, with no gnulib replacement as of yet. Do >> we need to add some HAVE_SYS_UN_H checks? > > Yes, i guess so. > >> >>> +#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). >>> +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. >>> + 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. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list