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 ? 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. > 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 > > int virSocketAddr(const char *str, struct sockaddr_storage *ss) > { The point of the exercise was to make some checks on the addresses given as string and passed down as string to an external tool > int len; > struct addrinfo hints = { 0 }; > struct addrinfo *res = NULL; > > hints.ai_flags = AI_NUMERICHOST; > > if (getaddrinfo(str, NULL, &hints, &res) != 0 || !res) > return -1; > > len = res->addrlen; > memcpy(ss, res->ai_addr, len); > > freeaddrinfo(res); > return len; > } > > 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. */ }; 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. > It is probably worth introducing a 'src/util/socketaddr.h' file > for this, since I think we'll need more socket addr functions > later No idea you had that in mind, go for it ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list