picnoir/Systemd
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

sd-dhcp: refactor dhcp_option_append

Browse source 
Store a pointer to the options in the DHCPMessage struct, and pass
this together with an offset around, rather than a uint8_t**.

This avoids us having to (re)compute the pointer; and changes
dhcp_option_append from adjusting both the pointer to the next
option and the remaining size of the options, to just adjusting
the current offset.

This makes the code a bit simpler to follow IMHO, but there should
be no functional change.
This commit is contained in:
Tom Gundersen 2014-05-20 11:04:50 +02:00
parent ece6e766cf
commit 20b958bf15
6 changed files with 74 additions and 96 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/libsystemd-network/dhcp-internal.h
View file

@ -36,8 +36,8 @@ int dhcp_network_send_raw_socket(int s, const union sockaddr_union *link,
int dhcp_network_send_udp_socket(int s, be32_t address, uint16_t port, int dhcp_network_send_udp_socket(int s, be32_t address, uint16_t port,
const void *packet, size_t len); const void *packet, size_t len);
int dhcp_option_append(uint8_t **buf, size_t *buflen, uint8_t code, int dhcp_option_append(uint8_t options[], size_t size, size_t *offset,
size_t optlen, const void *optval); uint8_t code, size_t optlen, const void *optval);
typedef int (*dhcp_option_cb_t)(uint8_t code, uint8_t len, typedef int (*dhcp_option_cb_t)(uint8_t code, uint8_t len,
const uint8_t *option, void *user_data); const uint8_t *option, void *user_data);
@ -46,7 +46,7 @@ int dhcp_option_parse(DHCPMessage *message, size_t len,
dhcp_option_cb_t cb, void *user_data); dhcp_option_cb_t cb, void *user_data);
int dhcp_message_init(DHCPMessage *message, uint8_t op, uint32_t xid, uint8_t type, int dhcp_message_init(DHCPMessage *message, uint8_t op, uint32_t xid, uint8_t type,
uint8_t **opt, size_t *optlen); uint8_t options[], size_t optlen, size_t *optoffset);
uint16_t dhcp_packet_checksum(void *buf, size_t len); uint16_t dhcp_packet_checksum(void *buf, size_t len);

33
src/libsystemd-network/dhcp-option.c
View file

@ -26,37 +26,33 @@
#include "dhcp-internal.h" #include "dhcp-internal.h"
int dhcp_option_append(uint8_t **buf, size_t *buflen, uint8_t code, int dhcp_option_append(uint8_t options[], size_t size, size_t *offset,
size_t optlen, const void *optval) uint8_t code, size_t optlen, const void *optval) {
{ assert(options);
if (!buf || !buflen) assert(offset);
return -EINVAL;
switch (code) { switch (code) {
case DHCP_OPTION_PAD: case DHCP_OPTION_PAD:
case DHCP_OPTION_END: case DHCP_OPTION_END:
if (*buflen < 1) if (size - *offset < 1)
return -ENOBUFS; return -ENOBUFS;
(*buf)[0] = code; options[*offset] = code;
*buf += 1; *offset += 1;
*buflen -= 1;
break; break;
default: default:
if (*buflen < optlen + 2) if (size - *offset < optlen + 2)
return -ENOBUFS; return -ENOBUFS;
if (!optval) assert(optval);
return -EINVAL;
(*buf)[0] = code; options[*offset] = code;
(*buf)[1] = optlen; options[*offset + 1] = optlen;
memcpy(&(*buf)[2], optval, optlen); memcpy(&options[*offset + 2], optval, optlen);
*buf += optlen + 2; *offset += optlen + 2;
*buflen -= (optlen + 2);
break; break;
} }
@ -143,7 +139,6 @@ int dhcp_option_parse(DHCPMessage *message, size_t len,
{ {
uint8_t overload = 0; uint8_t overload = 0;
uint8_t message_type = 0; uint8_t message_type = 0;
uint8_t *opt = (uint8_t *)(message + 1);
int res; int res;
if (!message) if (!message)
@ -154,7 +149,7 @@ int dhcp_option_parse(DHCPMessage *message, size_t len,
len -= sizeof(DHCPMessage); len -= sizeof(DHCPMessage);
res = parse_options(opt, len, &overload, &message_type, res = parse_options(message->options, len, &overload, &message_type,
cb, user_data); cb, user_data);
if (res < 0) if (res < 0)
return res; return res;

15
src/libsystemd-network/dhcp-packet.c
View file

@ -38,8 +38,9 @@
#define DHCP_CLIENT_MIN_OPTIONS_SIZE 312 #define DHCP_CLIENT_MIN_OPTIONS_SIZE 312
int dhcp_message_init(DHCPMessage *message, uint8_t op, uint32_t xid, int dhcp_message_init(DHCPMessage *message, uint8_t op, uint32_t xid,
uint8_t type, uint8_t **opt, size_t *optlen) { uint8_t type, uint8_t options[], size_t optlen, size_t *optoffset) {
int err; size_t offset = 0;
int r;
assert(op == BOOTREQUEST || op == BOOTREPLY); assert(op == BOOTREQUEST || op == BOOTREPLY);
@ -49,12 +50,12 @@ int dhcp_message_init(DHCPMessage *message, uint8_t op, uint32_t xid,
message->xid = htobe32(xid); message->xid = htobe32(xid);
message->magic = htobe32(DHCP_MAGIC_COOKIE); message->magic = htobe32(DHCP_MAGIC_COOKIE);
*opt = (uint8_t *)(message + 1); r = dhcp_option_append(message->options, optlen, &offset,
DHCP_OPTION_MESSAGE_TYPE, 1, &type);
if (r < 0)
return r;
err = dhcp_option_append(opt, optlen, DHCP_OPTION_MESSAGE_TYPE, 1, *optoffset = offset;
&type);
if (err < 0)
return err;
return 0; return 0;
} }

1
src/libsystemd-network/dhcp-protocol.h
View file

@ -44,6 +44,7 @@ struct DHCPMessage {
uint8_t sname[64]; uint8_t sname[64];
uint8_t file[128]; uint8_t file[128];
be32_t magic; be32_t magic;
uint8_t options[0];
} _packed_; } _packed_;
typedef struct DHCPMessage DHCPMessage; typedef struct DHCPMessage DHCPMessage;

57
src/libsystemd-network/sd-dhcp-client.c
View file

@ -257,19 +257,18 @@ static sd_dhcp_client *client_stop(sd_dhcp_client *client, int error) {
} }
static int client_message_init(sd_dhcp_client *client, DHCPMessage *message, static int client_message_init(sd_dhcp_client *client, DHCPMessage *message,
uint8_t type, uint8_t **opt, size_t *optlen) { uint8_t type, size_t optlen, size_t *optoffset) {
be16_t max_size; be16_t max_size;
int r; int r;
assert(client); assert(client);
assert(client->secs); assert(client->secs);
assert(message); assert(message);
assert(opt); assert(optoffset);
assert(optlen);
assert(type == DHCP_DISCOVER || type == DHCP_REQUEST); assert(type == DHCP_DISCOVER || type == DHCP_REQUEST);
r = dhcp_message_init(message, BOOTREQUEST, client->xid, type, opt, r = dhcp_message_init(message, BOOTREQUEST, client->xid, type,
optlen); message->options, optlen, optoffset);
if (r < 0) if (r < 0)
return r; return r;
@ -285,7 +284,8 @@ static int client_message_init(sd_dhcp_client *client, DHCPMessage *message,
/* Some DHCP servers will refuse to issue an DHCP lease if the Client /* Some DHCP servers will refuse to issue an DHCP lease if the Client
Identifier option is not set */ Identifier option is not set */
r = dhcp_option_append(opt, optlen, DHCP_OPTION_CLIENT_IDENTIFIER, r = dhcp_option_append(message->options, optlen, optoffset,
DHCP_OPTION_CLIENT_IDENTIFIER,
sizeof(client->client_id), &client->client_id); sizeof(client->client_id), &client->client_id);
if (r < 0) if (r < 0)
return r; return r;
@ -299,10 +299,9 @@ static int client_message_init(sd_dhcp_client *client, DHCPMessage *message,
it MUST include that list in any subsequent DHCPREQUEST it MUST include that list in any subsequent DHCPREQUEST
messages. messages.
*/ */
r = dhcp_option_append(opt, optlen, r = dhcp_option_append(message->options, optlen, optoffset,
DHCP_OPTION_PARAMETER_REQUEST_LIST, DHCP_OPTION_PARAMETER_REQUEST_LIST,
client->req_opts_size, client->req_opts_size, client->req_opts);
client->req_opts);
if (r < 0) if (r < 0)
return r; return r;
@ -316,7 +315,7 @@ static int client_message_init(sd_dhcp_client *client, DHCPMessage *message,
*/ */
max_size = htobe16(DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE + max_size = htobe16(DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE +
DHCP_MIN_OPTIONS_SIZE); DHCP_MIN_OPTIONS_SIZE);
r = dhcp_option_append(opt, optlen, r = dhcp_option_append(message->options, optlen, optoffset,
DHCP_OPTION_MAXIMUM_MESSAGE_SIZE, DHCP_OPTION_MAXIMUM_MESSAGE_SIZE,
2, &max_size); 2, &max_size);
if (r < 0) if (r < 0)
@ -336,8 +335,7 @@ static int dhcp_client_send_raw(sd_dhcp_client *client, DHCPPacket *packet,
static int client_send_discover(sd_dhcp_client *client) { static int client_send_discover(sd_dhcp_client *client) {
_cleanup_free_ DHCPPacket *discover = NULL; _cleanup_free_ DHCPPacket *discover = NULL;
size_t optlen, len; size_t optoffset, optlen, len;
uint8_t *opt;
usec_t time_now; usec_t time_now;
int r; int r;
@ -364,7 +362,7 @@ static int client_send_discover(sd_dhcp_client *client) {
return -ENOMEM; return -ENOMEM;
r = client_message_init(client, &discover->dhcp, DHCP_DISCOVER, r = client_message_init(client, &discover->dhcp, DHCP_DISCOVER,
&opt, &optlen); optlen, &optoffset);
if (r < 0) if (r < 0)
return r; return r;
@ -375,22 +373,21 @@ static int client_send_discover(sd_dhcp_client *client) {
option to suggest the lease time it would like. option to suggest the lease time it would like.
*/ */
if (client->last_addr != INADDR_ANY) { if (client->last_addr != INADDR_ANY) {
r = dhcp_option_append(&opt, &optlen, r = dhcp_option_append(discover->dhcp.options, optlen, &optoffset,
DHCP_OPTION_REQUESTED_IP_ADDRESS, DHCP_OPTION_REQUESTED_IP_ADDRESS,
4, &client->last_addr); 4, &client->last_addr);
if (r < 0) if (r < 0)
return r; return r;
} }
r = dhcp_option_append(&opt, &optlen, DHCP_OPTION_END, 0, NULL); r = dhcp_option_append(discover->dhcp.options, optlen, &optoffset,
if (r < 0) DHCP_OPTION_END, 0, NULL);
return r;
/* We currently ignore: /* We currently ignore:
The client SHOULD wait a random time between one and ten seconds to The client SHOULD wait a random time between one and ten seconds to
desynchronize the use of DHCP at startup. desynchronize the use of DHCP at startup.
*/ */
r = dhcp_client_send_raw(client, discover, len - optlen); r = dhcp_client_send_raw(client, discover, sizeof(DHCPPacket) + optoffset);
if (r < 0) if (r < 0)
return r; return r;
@ -401,8 +398,7 @@ static int client_send_discover(sd_dhcp_client *client) {
static int client_send_request(sd_dhcp_client *client) { static int client_send_request(sd_dhcp_client *client) {
_cleanup_free_ DHCPPacket *request; _cleanup_free_ DHCPPacket *request;
size_t optlen, len; size_t optoffset, optlen, len;
uint8_t *opt;
int r; int r;
optlen = DHCP_MIN_OPTIONS_SIZE; optlen = DHCP_MIN_OPTIONS_SIZE;
@ -412,8 +408,8 @@ static int client_send_request(sd_dhcp_client *client) {
if (!request) if (!request)
return -ENOMEM; return -ENOMEM;
r = client_message_init(client, &request->dhcp, DHCP_REQUEST, &opt, r = client_message_init(client, &request->dhcp, DHCP_REQUEST,
&optlen); optlen, &optoffset);
if (r < 0) if (r < 0)
return r; return r;
@ -428,13 +424,13 @@ static int client_send_request(sd_dhcp_client *client) {
filled in with the yiaddr value from the chosen DHCPOFFER. filled in with the yiaddr value from the chosen DHCPOFFER.
*/ */
r = dhcp_option_append(&opt, &optlen, r = dhcp_option_append(request->dhcp.options, optlen, &optoffset,
DHCP_OPTION_SERVER_IDENTIFIER, DHCP_OPTION_SERVER_IDENTIFIER,
4, &client->lease->server_address); 4, &client->lease->server_address);
if (r < 0) if (r < 0)
return r; return r;
r = dhcp_option_append(&opt, &optlen, r = dhcp_option_append(request->dhcp.options, optlen, &optoffset,
DHCP_OPTION_REQUESTED_IP_ADDRESS, DHCP_OPTION_REQUESTED_IP_ADDRESS,
4, &client->lease->address); 4, &client->lease->address);
if (r < 0) if (r < 0)
@ -447,7 +443,7 @@ static int client_send_request(sd_dhcp_client *client) {
option MUST be filled in with client’s notion of its previously option MUST be filled in with client’s notion of its previously
assigned address. ’ciaddr’ MUST be zero. assigned address. ’ciaddr’ MUST be zero.
*/ */
r = dhcp_option_append(&opt, &optlen, r = dhcp_option_append(request->dhcp.options, optlen, &optoffset,
DHCP_OPTION_REQUESTED_IP_ADDRESS, DHCP_OPTION_REQUESTED_IP_ADDRESS,
4, &client->last_addr); 4, &client->last_addr);
if (r < 0) if (r < 0)
@ -480,7 +476,8 @@ static int client_send_request(sd_dhcp_client *client) {
return -EINVAL; return -EINVAL;
} }
r = dhcp_option_append(&opt, &optlen, DHCP_OPTION_END, 0, NULL); r = dhcp_option_append(request->dhcp.options, optlen, &optoffset,
DHCP_OPTION_END, 0, NULL);
if (r < 0) if (r < 0)
return r; return r;
@ -489,9 +486,9 @@ static int client_send_request(sd_dhcp_client *client) {
client->lease->server_address, client->lease->server_address,
DHCP_PORT_SERVER, DHCP_PORT_SERVER,
&request->dhcp, &request->dhcp,
len - optlen - DHCP_IP_UDP_SIZE); sizeof(DHCPMessage) + optoffset);
} else { } else {
r = dhcp_client_send_raw(client, request, len - optlen); r = dhcp_client_send_raw(client, request, sizeof(DHCPPacket) + optoffset);
} }
if (r < 0) if (r < 0)
return r; return r;

58
src/libsystemd-network/test-dhcp-option.c
View file

@ -85,16 +85,14 @@ static void test_invalid_buffer_length(void)
static void test_message_init(void) static void test_message_init(void)
{ {
_cleanup_free_ DHCPMessage *message = NULL; _cleanup_free_ DHCPMessage *message = NULL;
size_t optlen = 3; size_t optlen = 3, optoffset;
size_t len = sizeof(DHCPMessage) + optlen; size_t len = sizeof(DHCPMessage) + optlen;
uint8_t *opt, *magic; uint8_t *magic;
message = malloc0(len); message = malloc0(len);
opt = (uint8_t *)(message + 1);
assert_se(dhcp_message_init(message, BOOTREQUEST, 0x12345678, assert_se(dhcp_message_init(message, BOOTREQUEST, 0x12345678,
DHCP_DISCOVER, &opt, &optlen) >= 0); DHCP_DISCOVER, message->options, optlen, &optoffset) >= 0);
assert_se(message->xid == htobe32(0x12345678)); assert_se(message->xid == htobe32(0x12345678));
assert_se(message->op == BOOTREQUEST); assert_se(message->op == BOOTREQUEST);
@ -115,13 +113,11 @@ static DHCPMessage *create_message(uint8_t *options, uint16_t optlen,
{ {
DHCPMessage *message; DHCPMessage *message;
size_t len = sizeof(DHCPMessage) + optlen; size_t len = sizeof(DHCPMessage) + optlen;
uint8_t *opt;
message = malloc0(len); message = malloc0(len);
opt = (uint8_t *)(message + 1);
if (options && optlen) if (options && optlen)
memcpy(opt, options, optlen); memcpy(&message->options, options, optlen);
if (file && filelen <= 128) if (file && filelen <= 128)
memcpy(&message->file, file, filelen); memcpy(&message->file, file, filelen);
@ -308,46 +304,34 @@ static uint8_t options[64] = {
static void test_option_set(void) static void test_option_set(void)
{ {
size_t len, oldlen; size_t offset = 0, len, pos;
int pos, i; unsigned i;
uint8_t *opt;
assert_se(dhcp_option_append(NULL, NULL, 0, 0, NULL) == -EINVAL); assert_se(dhcp_option_append(result, 0, &offset, DHCP_OPTION_PAD,
len = 0;
opt = &result[0];
assert_se(dhcp_option_append(&opt, NULL, 0, 0, NULL) == -EINVAL);
assert_se(opt == &result[0] && len == 0);
assert_se(dhcp_option_append(&opt, &len, DHCP_OPTION_PAD,
0, NULL) == -ENOBUFS); 0, NULL) == -ENOBUFS);
assert_se(opt == &result[0] && len == 0); assert_se(offset == 0);
opt = &result[4]; offset = 4;
len = 1; assert_se(dhcp_option_append(result, 1, &offset, DHCP_OPTION_PAD,
assert_se(dhcp_option_append(&opt, &len, DHCP_OPTION_PAD,
0, NULL) >= 0); 0, NULL) >= 0);
assert_se(opt == &result[5] && len == 0); assert_se(offset == 5);
pos = 4; offset = pos = 4;
len = 60; len = 60;
while (pos < 64 && options[pos] != DHCP_OPTION_END) { while (pos < 64 && options[pos] != DHCP_OPTION_END) {
opt = &result[pos]; offset = pos;
oldlen = len;
assert_se(dhcp_option_append(&opt, &len, options[pos], assert_se(dhcp_option_append(result, len, &offset,
options[pos + 1], options[pos],
&options[pos + 2]) >= 0); options[pos + 1],
&options[pos + 2]) >= 0);
if (options[pos] == DHCP_OPTION_PAD) { if (options[pos] == DHCP_OPTION_PAD)
assert_se(opt == &result[pos + 1]);
assert_se(len == oldlen - 1);
pos++; pos++;
} else { else
assert_se(opt == &result[pos + 2 + options[pos + 1]]);
assert_se(len == oldlen - 2 - options[pos + 1]);
pos += 2 + options[pos + 1]; pos += 2 + options[pos + 1];
}
assert_se(offset == pos);
} }
for (i = 0; i < pos; i++) { for (i = 0; i < pos; i++) {