| Re: [chrony-dev] nameserv.c unused #ifdef and getnameinfo result ignored |
[ Thread Index |
Date Index
| More chrony.tuxfamily.org/chrony-dev Archives
]
On Mon, 4 Jan 2010, Miroslav Lichvar wrote:
Bill Unruh wrote:
In the file nameserver.c in the function DNS_IPAddress2Name
the program starts out with an
#ifdef HAVE_IPV6 ....
#else
...
#ifdef HAVE_IPV6
...
#endif
...
#endif
That second block of #ifdef HAVE_IPV6 will never be used since it is in a
block that is run only if HAVE_IPV6 is not set.
That's ok, there are two code paths, one for getnameinfo and one for
gethostbyaddr. Both support IPv6, but the first one is currently
preferred if IPv6 support is enabled.
Except it is impossible to ever get into the second path.
If HAVE_IPV6 is defined, then the first block is run. If not then that second
block is never even compiled. Ie, it is impossible to get into the second
block.
Remember that #else really means #if ! HAVE_IPV6 .
Also, this routine simply accepts anything that the getnameinfo throws at it
without doing any error checking.
It checks for the return code. If there is an error, IPToString is
called to provide the output.
The problem is that for example the "buffer too small" error should not
produce that AFAIK.
In client.c:
Hmm, the hostname_buff in process_smd_clients and process_cmd_sources is only 32 bytes long, which is arguably too small for an ipv6
address, of which only 25 bytes are displayed-- is this enough to display a
unique ipv6 address to a source?
In process_cmd_tracking the buffer "host" is 50 bytes long instead which seems
more reasonable.
Good point, this should be fixed.
In fact there is a variable NI_MAXHOST which is the largest hostname possible,
which the man getnameinfo suggests one uses to define the size of the buffer.
( it is about 1000 bytes). That may be overdoing it.
Note that in process_cmd_tracking, the report seems to be aimed at IPV4, and
would fall apart with IPV6.
eg:
printf("Reference ID : %lu.%lu.%lu.%lu (%s)\n", a, b, c, d, ref_ip);
The reference ID is always 32 bits (for IPv6 address it's its MD5 sum)
and the IP address or refid as string for refclocks is printed in
parentheses. But I agree it may be confusing, suggestions welcomed.
Looking over the code again, there seems to be a problem in client.c function
parse_allow_deny with the function assuming that the address is an IPV4
address. See for example
if (!UTI_StringToIP(p, &ip) &&
(n = sscanf(p, "%lu.%lu.%lu.%lu", &a, &b, &c, &d)) == 0) {
which seems to assume that the address read in is IPV4.
That should be ok. The sscanf is called only when UTI_StringToIP fails
which will be only for incomplete IPv4 addresses like a.b.c or a.b.
For IPv6 addresses only the ::/bits notation may be used.
Ie, there seems to be something wrong in NetBSD with the getnameinfo function.
(That error message is pretty uninformative.)
After a bit of googling it looks like NetBSD requires sin_len and
sin6_len fiels to be set, I'll look into it later.
Great.
--
William G. Unruh | Canadian Institute for| Tel: +1(604)822-3273
Physics&Astronomy | Advanced Research | Fax: +1(604)822-5324
UBC, Vancouver,BC | Program in Cosmology | unruh@xxxxxxxxxxxxxx
Canada V6T 1Z1 | and Gravity | www.theory.physics.ubc.ca/
---
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble? Email listmaster@xxxxxxxxxxxxxxxxxxxx.