[haiku-commits] Re: r40392 - haiku/trunk/src/tests/kits/net/preflet/InterfacesAddOn
- From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Tue, 08 Feb 2011 20:31:43 +0100
kallisti5@xxxxxxxxxxx wrote:
> @@ -107,6 +97,9 @@
> BBitmap* stateIcon(NULL);
> BString interfaceState;
>
> + BNetworkAddress addrIPv4 = fSettings->GetAddr(AF_INET);
> + BNetworkAddress addrIPv6 = fSettings->GetAddr(AF_INET6);
> +
> if (!list)
> return;
We don't put anything but a single space between the type and the
variable. Besides this, this code looks odd; variables should be
declared where they are needed, not at the top of functions.
Especially when the condition could easily be checked before.
> - ifreq request;
> - if (!_PrepareRequest(request))
> + ifreq ifReq;
> + sockaddr_in* inetAddress = NULL;
> +
> + if (!_PrepareRequest(ifReq))
> return;
Same issues here, besides that I like 'request' better than ifReq.
> + fIPv4Addr = *((sockaddr_in *)(&(ifReq.ifr_addr)));
Way too many parenthesis here - a single pair is all you need in this
line.
You do this again later in the code.
BTW I can recommend switching to BNetworkInterface instead - this makes
all that code a whole lot more readable, and is also a lot easier to
use and maintain.
> - ifconf config;
> - config.ifc_len = sizeof(config.ifc_value);
> - if (ioctl(fSocket, SIOCGRTSIZE, &config, sizeof(struct ifconf)) <
> 0)
> + ifconf ifCfg;
Same here, "ifCfg" is not a nice name; either resemble the structure
name as you did with ifReq, or use real words like
interfaceConfiguration if you want to be verbose.
IMO 'config' is a much better compromise, though.
> +BNetworkAddress
> +NetworkSettings::GetAddr(int family)
I haven't looked closely, but it looks like that class could just return
BNetworkAddresses directly instead.
> +const char*
> +NetworkSettings::GetIP(int family)
[...]
> +const char*
> +NetworkSettings::GetNetmask(int family)
[...]
> +int32
> +NetworkSettings::GetPrefixLen(int family)
Besides that, you changed the correct naming scheme (without the Get*)
to one that doesn't match our other classes.
Get*() should only be used if the operation can fail, and you don't
directly return the object (but use a reference parameter instead).
Bye,
Axel.
Other related posts:
