Thanks for the patch!<br><br>Some comments:<br><br>+ host_str = malloc(value_len + 1);
<br>+ strncpy(host_str, spec, value_len);<br><br>Here you are not checking the return value of malloc(). <br><br>Oh, I see that the old code that this is replacing didn't check the return value of strdup() either.<br>
So I guess this is no worse.<br><br>+static void set_allones(dcc_address_t *addr)
<br>+{
<br>+#ifndef ENABLE_RFC2553
<br>+ *addr = *(dcc_address_t *)&allones;<br><br>I'm concerned that this code may violate the C standard's rules on aliasing/type-punning.<br><br>+static void set_mask_inet6(struct in6_addr *mask, int mask_bits) {
<br>+ uint32_t *iptr = (uint32_t *)mask;
<br>+ int allones_count = mask_bits / 32;
<br>+ int i;
<br>+
<br>+ for(i = 0; i < allones_count; ++i, ++iptr)
<br>+ *iptr = allones;
<br>+
<br>+ set_mask_32(iptr, mask_bits % 32);
<br>+ ++iptr;
<br>+
<br>+ for(i = 4 - allones_count - 1; i > 0; --i, ++iptr)
<br>+ *iptr = 0;
<br>+}<br><br>Likewise here.<br><br>The C standard says: (ISO/IEC 9899:TC2, section 6.5, paragraph 7):<br><br><div style="margin-left: 40px;">An object shall have its stored value accessed only by an lvalue expression that has one of<br>
the following types:74)<br>-- a type compatible with the effective type of the object,<br>-- a qualified version of a type compatible with the effective type of the object,<br>-- a type that is the signed or unsigned type corresponding to the effective type of the<br>
object,<br>-- a type that is the signed or unsigned type corresponding to a qualified version of the<br> effective type of the object,<br>-- an aggregate or union type that includes one of the aforementioned types among its<br>
members (including, recursively, a member of a subaggregate or contained union), or<br>-- a character type.<br></div><br>The Posix standard says:<br><p style="margin-left: 40px;"><sup>[<a href="javascript:open_code('IP6')">IP6</a>]</sup> <img src="http://www.opengroup.org/onlinepubs/000095399/images/opt-start.gif" alt="[Option Start]" border="0"> The <i><netinet/in.h></i> header shall define the <b>in6_addr</b> structure that contains at least the following
member:</p>
<pre style="margin-left: 40px;"><tt>uint8_t s6_addr[16]</tt><br></pre>
<div class="gmail_quote">So in general I don't think it is standard-conforming to assume that an in6_addr can be treated as an array of uint32_t.<br><br>I think that this issue could be avoided by rewriting the code using
uint8_t instead of uint32_t and using the s6_addr member of the
in6_addr struct instead of casts.<br><br>Now in practice the code probably works fine. So maybe I'm being a bit pedantic here.<br>But I think it could be made standard-conforming without that much additional effort.<br>
What do you think?<br><br>Cheers,<br> Fergus.<br><br>On Mon, Jul 13, 2009 at 4:52 AM, Bob Ham <span dir="ltr"><rah@bash.sh></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi there,<br>
<br>
The attached patch implements IPv6 support for access control. It also<br>
includes a small update to restrict zeroconf advertisements to IPv4 if<br>
RFC2553 support isn't enabled.<br>
<br>
I've tested the patch both with RFC2553 support enabled and without.<br>
There seem to be no regressions.<br>
<br>
A more extensive zeroconf update will follow to restrict the addresses<br>
it advertises to ones that are actually listened on. It seems there may<br>
be issues listening on IPv6 addresses as well.<br>
<br>
Regards,<br>
<br>
Bob Ham<br>
<font color="#888888"><br>
--<br>
Bob Ham <rah@bash.sh><br>
<br>
for (;;) { ++pancakes; }<br>
</font><br>__<br>
distcc mailing list <a href="http://distcc.samba.org/" target="_blank">http://distcc.samba.org/</a><br>
To unsubscribe or change options:<br>
<a href="https://lists.samba.org/mailman/listinfo/distcc" target="_blank">https://lists.samba.org/mailman/listinfo/distcc</a><br></blockquote></div><br><br clear="all"><br>-- <br>Fergus Henderson <<a href="mailto:fergus@google.com">fergus@google.com</a>><br>