Hi Marcel,

On 10.10.2012 18:10, Marcel Holtmann wrote:
Hi Jukka,

Convert to network byte order just before sending the packet.
---
  gdhcp/client.c | 47 +++++++++++++++++++++--------------------------
  gdhcp/common.c | 20 +++++++++++++++-----
  gdhcp/common.h | 17 +----------------
  gdhcp/ipv4ll.c |  4 ++--
  gdhcp/server.c | 41 +++++++++++++++++++----------------------
  5 files changed, 58 insertions(+), 71 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index cf04ced..7340f3a 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -367,7 +367,7 @@ static int send_discover(GDHCPClient *dhcp_client, uint32_t 
requested)

        /* Explicitly saying that we want RFC-compliant packets helps
         * some buggy DHCP servers to NOT send bigger packets */
-       dhcp_add_simple_option(&packet, DHCP_MAX_SIZE, htons(576));
+       dhcp_add_simple_option(&packet, DHCP_MAX_SIZE, 576);

so here is a uint16 ...

        add_request_options(dhcp_client, &packet);

@@ -410,7 +410,7 @@ static int send_renew(GDHCPClient *dhcp_client)

        init_packet(dhcp_client , &packet, DHCPREQUEST);
        packet.xid = dhcp_client->xid;
-       packet.ciaddr = dhcp_client->requested_ip;
+       packet.ciaddr = htonl(dhcp_client->requested_ip);

        add_request_options(dhcp_client, &packet);

@@ -429,7 +429,7 @@ static int send_rebound(GDHCPClient *dhcp_client)

        init_packet(dhcp_client , &packet, DHCPREQUEST);
        packet.xid = dhcp_client->xid;
-       packet.ciaddr = dhcp_client->requested_ip;
+       packet.ciaddr = htonl(dhcp_client->requested_ip);

        add_request_options(dhcp_client, &packet);

@@ -449,7 +449,7 @@ static int send_release(GDHCPClient *dhcp_client,

        init_packet(dhcp_client, &packet, DHCPRELEASE);
        packet.xid = rand();
-       packet.ciaddr = ciaddr;
+       packet.ciaddr = htonl(ciaddr);

        dhcp_add_simple_option(&packet, DHCP_SERVER_ID, server);

... and here a uint32. I do not like this kind of magic API where a
uint32 gets converted to whatever it needs to be.

Situation is probably not so bad as the size of the used data is known beforehand. See client_options struct in common.c. So in dhcp_add_simple_option() we definitely know the size (which can be either 1, 2 or 4).


The caller should now what size it is. So we need dhcp_add_option_uint16
and dhcp_add_option_uint32 and so on. Lets make this clearly typed and
then clang will also complain less.

You realize that I am not a fan of magic casting data types into place.
That just hides bugs.

Regards

Marcel


Cheers,
Jukka


_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman

Reply via email to