On Thu, Jul 23, 2015 at 12:28:33PM +0200, Pavel Reichl wrote:
> On 07/23/2015 11:42 AM, Jakub Hrozek wrote:
> >+static errno_t match_ip(const struct sockaddr *sa,
> >+                        const struct sockaddr *sb,
> >+                        bool *_res)
> >>>Don't you think it would be simpler to just return bool? For cases where
> >>Yes, it would be simpler, but I prefer to know that address is from family
> >>that is not supported by the function. But I can change it if you still
> >>prefer that.
> >>
> >>>the address family is totally different, can you just return false?
> >>I actually think this is how function works (at least for address families
> >>known to function)
> >>>>+{
> >>>>+    size_t addrsize;
> >>>>+    bool res;
> >>>>+    errno_t ret;
> >>>>+    const void *addr_a;
> >>>>+    const void *addr_b;
> >>>>+
> >>>>+    if (sa->sa_family == AF_INET) {
> >>>>+        addrsize = sizeof(struct in_addr);
> >>>>+        addr_a = (const void *) &((const struct sockaddr_in *) 
> >>>>sa)->sin_addr;
> >>>>+        addr_b = (const void *) &((const struct sockaddr_in *) 
> >>>>sb)->sin_addr;
> >>>>+    } else if (sa->sa_family == AF_INET6) {
> >>>>+        addrsize = sizeof(struct in6_addr);
> >>>>+        addr_a = (const void *) &((const struct sockaddr_in6 *) 
> >>>>sa)->sin6_addr;
> >>>>+        addr_b = (const void *) &((const struct sockaddr_in6 *) 
> >>>>sb)->sin6_addr;
> >>>Hmm, I was surprised I couldn't find any existing function or macro to
> >>>compare a sockaddr or a sockaddr_storage except IN6_ARE_ADDR_EQUAL..
> >>>
> >>>>+    } else {
> >>>>+        ret = EINVAL;
> >>>>+        goto done;
> >>unsupported address family
> >I was thinking just return false here and skip that IP, just like for
> >different families and skip the invalid address...
> OK, I'll do it as you propose.
> >
> >btw what other address can there be on an interface?
> I honestly don't know, that's why I tried to be careful about it.

Well, do you see any risk if there was an invalid address and we skipped
it?

Is there a test that would try the nothing matched scenario? If yes,
then I would prefer to simplify the code, otherwise keep your more
paranoid less readable version.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to