Re: [PATCH 1/7] virutil: Introduce virIsValidHostname

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 20, 2015 at 15:25:29 -0400, John Ferlan wrote:
> 
> 
> >> As a hindsight from reviewing 6/7. This function should also be in
> >> virsocketaddr.c
> >>
> >>
> > 
> > 
> > hmmm.. yes I see.. Guess I got hung up on "virSocketAddr..." and didn't
> > focus as closely on the implementation where virSocketAddrParse can take
> > NULL as the first parameter... Guess, that means patches 2-5 can just be
> > called as:
> > 
> >     if (virSocketAddrParse(NULL, pool->def->source.hosts[0].name,
> > AF_UNSPEC) < 0)
> > 
> > 
> 
> Without actually trying it - seemed like a good idea; however, the 
> virSocketAddrParse uses/sets "hints.ai_flags = AI_NUMERICHOST;" thus
> it requires a numeric value and not one that could be a name or a
> number, so it seems this particular code cannot use it.

Well, you still can add a new helper that will allow to use your
approach.

> 
> I really see those virSocketAddr* API's as different, very specific
> to supporting the network socket's and socket address formats; whereas,
> this code will take a string representation of either the name or
> the number as provided in the XML and validate it.

We still can make it a more universal helper for address conversions.

> 
> I don't think this set of API's belongs there as it's not manipulating
> virSocketAddr's.

The general utils file used to be a dumping place for all the various
helpers, but now we are usually trying to put them together with the
semanticaly similar helpers, or making a group of them to make them
semantically similar. I think it makes sense in this case and the
general utils file is not the right place for this helper.

> So, I'll change the function intro to:
> 
>  * Unlike virGetHostname, this variant of the code receives a hostname and
>  * retrieves the getaddrinfo. If the passed hostname can be successfully
>  * resolved via getaddrinfo, then return true; otherwise, if the hostname
>  * cannot be resolved for any reason, return false.
> 
> and 
> 
> remove the localhost specific checking
> 
> and adjust the commit message to remove the 'getnameinfo' reference.
> 
> John

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]