Re: [PATCH] dnsmasq: does handle more than one nameserver per domain

[Jack Bates <u32lyv nottheoilrig com>]

On 09/07/13 03:10 PM, Pavel Simerda wrote:
> > From: "Jack Bates" <u32lyv nottheoilrig com>
> > On 09/07/13 01:50 AM, Pavel Simerda wrote:
> > > Hi,
> > >
> > > +1 from me with a couple of minor issues:
> >
> > Thank you Pavel, I attempted to address the issues:
> >
> > > * Name *i* is good enough for an integer counter.
> >
> > There are already some loops using *i* and if I'm not mistaken, they
> > would conflict. The patch nests existing loops (searches and domains) in
> > a new loop (nameservers). Do you have other advice for this integer counter?
>
> I tend to just use i, j, k, but that's not a real issue.
>
> > > * We usually use 'nnameservers' and the like, not 'n_nameserver'.
> >
> > Fixed. I updated the patch:
> >
> >      http://nottheoilrig.com/networkmanager/201307090/perdomain.patch
> >
> > > * I recommend using in_addr_t (or guint32) as we do in most of the code for
> > > IPv4 addresses instead of struct in_addr.
> >
> > The existing code invokes inet_ntop to format the address as a string,
> > and I think inet_ntop expects an in_addr struct. Is there a preferred
> > way to format an ip4 address as a string? (I took a quick look through
> > the code but didn't find an alternative way.)
>
> Actually, you can just typecast in_addr_t to struct in_addr and vice versa, as the latter is technically just 
> a structure containing the former. And in the case of inet_ntop, you don't even need to typecast as it expect 
> void *.

Oh, I see (sorry I didn't realize that). I added a commit to change from 
using struct in_addr to in_addr_t:

   http://nottheoilrig.com/networkmanager/201307100/perdomain.patch

Thank you for reviewing this, is there anything else I should do?