Re: [libvirt] [PATCH] 1/3 Add IPv4 and IPv6 parsing routines
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/