[haiku-commits] Re: r36192 - in haiku/trunk: headers/private/net src/add-ons/kernel/network/protocols/ipv4 src/add-ons/kernel/network/protocols/l2cap src/add-ons/kernel/network/protocols/tcp src/add-ons/kernel/network/protocols/udp ...
- From: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Tue, 13 Apr 2010 00:08:32 +0200
On 04/12/2010 08:39 PM, zooey@xxxxxxxxxxxxxxx wrote:
> Log:
> Applying patch by Atis Elsts:
Pointing out mostly style issues:
> + bool (*is_same_family)(const sockaddr *address);
Not sure if that's really needed; the sa_family is part of sockaddr, and
the AF_* family is known, too (ie. the domain knows it).
> --- haiku/trunk/src/add-ons/kernel/network/protocols/ipv4/ipv4_address.cpp
> 2010-04-12 18:21:59 UTC (rev 36191)
> +++ haiku/trunk/src/add-ons/kernel/network/protocols/ipv4/ipv4_address.cpp
> 2010-04-12 18:39:34 UTC (rev 36192)
> @@ -107,7 +107,19 @@
> && (!checkPort || ((sockaddr_in *)address)->sin_port == 0);
> }
>
> +/*! Checks if the given \a address is an IPv4 address.
Two blank lines.
> + \return false if \a address is NULL, or with family different from
> AF_INET
> + true if it has AF_INET address family
[...]
> +}
> +
> /*! Compares the IP-addresses of the two given address structures \a a and
> \a b.
And here again, and at most other changes, too.
> - // TODO: this is IPv4 specific, and doesn't belong here!
> // consider destination address INADDR_ANY as INADDR_LOOPBACK
> - sockaddr_in _address;
> - if (((sockaddr_in*)address)->sin_addr.s_addr == INADDR_ANY) {
> - memcpy(&_address, address, sizeof(sockaddr_in));
> - _address.sin_len = sizeof(sockaddr_in);
> - _address.sin_addr.s_addr = INADDR_LOOPBACK;
> - address = (sockaddr*)&_address;
> + sockaddr_storage _address;
> + if (AddressModule()->is_empty_address(address, false)) {
> + AddressModule()->get_loopback_address((sockaddr *)&_address);
> + // for IPv4 and IPv6 the port is at the same offset
> + ((sockaddr_in &)_address).sin_port = ((sockaddr_in
> *)address)->sin_port;
> + address = (sockaddr *)&_address;
This is a pretty ugly solution - being address family agnostic TCP
should use the set_port() call of the address module instead.
Also, this introduces an inconsistent asterisk style.
> + // consider destination address INADDR_ANY as INADDR_LOOPBACK
> + sockaddr_storage _address;
> + if (AddressModule()->is_empty_address(address, false)) {
> + AddressModule()->get_loopback_address((sockaddr
> *)&_address);
> + // for IPv4 and IPv6 the port is at the same offset
> + ((sockaddr_in&)_address).sin_port
> + = ((sockaddr_in *)address)->sin_port;
> + address = (sockaddr *)&_address;
> + }
Same here, besides the asterisk style :-)
> --- haiku/trunk/src/tests/kits/net/ipv46_client.cpp
> (rev 0)
> +++ haiku/trunk/src/tests/kits/net/ipv46_client.cpp 2010-04-12 18:39:34 UTC
> (rev 36192)
> @@ -0,0 +1,88 @@
> +#include <unistd.h>
> +#include <memory.h>
Always nice to have the author and a copyright notice in a file.
> +#include <netinet/in.h>
> +#include <arpa/inet.h>
> +#include <errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +const unsigned short TEST_PORT = 40000;
> +
> +void usage() {
> + printf("client [tcp|udp] [4|6] [4|6]\n");
> + exit(1);
> +}
> +
> +int main(int argc, char *argv[]) {
Coding style shouldn't be ignored for test apps either.
Bye,
Axel.
Other related posts: