On Wed, Oct 14, 2009 at 12:51:32PM +0200, Daniel Veillard wrote: > On Wed, Oct 14, 2009 at 10:49:20AM +0100, Daniel P. Berrange wrote: > > On Tue, Oct 13, 2009 at 04:12:47PM +0200, Daniel Veillard wrote: > > > * src/util/util.h src/util/util.c: two new functions virParseIPv4 > > > and virParseIPv6 > > > > I think this should just be a thin wrapper around getaddrinfo() > > which already knows how to parse all possible address types. > > Are we allowing all possible address types in the XML ? Well at this time we only allow IPv4 addreses in this, so we would want to pass the AF_INET flag to restrict it. Other areas of libvirt which could uses this code would be more flexible. We've got an open RFE for IPv6 support https://bugzilla.redhat.com/show_bug.cgi?id=514749 > I based that parsing routing in a large part as a syntactic check > on the allowed set of addresses. For example I didn't allow > > ::ffff:12.34.56.78 > > kind of IPv6 syntax. Why shouldn't we allow that - its valid IPv6 address syntax } > > If we avoid a custom typedef, and just use 'struct sockaddr_storage' > > this in turn makes it easy for callers to pass the result straight > > into any socket API calls they might use. eg this could all be done > > with a couple of lines of code > > > > > > That would automatically cope with both IPv4 / 6 addresses. If > > we want to restrict it we could add a 3rd argument, 'int family' > > and use that to set hints.ai_family field - the caller would > > just pass AF_INET or AF_INET6 to specify a particular type, or > > leave it at 0 to allow any type. > > Dunno, I find the getaddrinfo interface and the hints stuff > fairly incomprehensible. When I see the OpenGroup definition of > struct sockaddr_storage > > struct sockaddr_storage { > sa_family_t ss_family; /* Address family. */ > /* > * Following fields are implementation-defined. > */ > char _ss_pad1[_SS_PAD1SIZE]; > /* 6-byte pad; this is to make implementation-defined > pad up to alignment field that follows explicit in > the data structure. */ > int64_t _ss_align; /* Field to force desired structure > storage alignment. */ > char _ss_pad2[_SS_PAD2SIZE]; > /* 112-byte pad to achieve desired size, > _SS_MAXSIZE value minus size of ss_family > __ss_pad1, __ss_align fields is 112. */ > }; NB it is not intended to use the internals of the sockaddr_storage struct, with the exception of the ss_family field. It is provided as a struct that is guarenteed large enough to store any type of address. To use the data, you'd cast to the appropriate struct based on ss_family. eg, if ss_family == AF_INET, then you can cast to struct sockaddr_in which has a 'struct in_addr sin_addr' field available to get the raw address - in this case 'in_addr' is just an int32 For AF_INET6, you would cast to sockaddr_in6, which has a 'struct in6_addr sin6_addr' which gives you easy acess to the 16 byte array of the address. > I'm not too enthusiastic about using this for internal APIs. > And I don't see how I would check ranges for the IP addresses based > on this. Actually I don't find it helps to calculate a range, > and I prefer my good old arrays of well defined ints for that purpose. The sockaddr_in/in6 structs both give you an array of bytes in exactly the same manner Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list