fill_one() previously returned true on overflow, but callers
used this to log a skip message per option — which becomes
noisy if the options field is full.
Instead, rely on opts[].sent (already set by fill_one()) to
detect unsent options: a final reporting loop walks all
options and logs a single skip message for each one that
couldn't be sent. This also makes the return value
unnecessary, so change fill_one() to return void.
Link: https://bugs.passt.top/show_bug.cgi?id=192
Signed-off-by: Anshu Kumari
---
v7:
- (new patch) Split out from the option overload patch into its own commit
---
dhcp.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/dhcp.c b/dhcp.c
index 4dccae7b..6f69fd62 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -418,16 +418,14 @@ const char *dhcp_opt_to_str(uint8_t code, char *buf, size_t buf_len)
* @size: Usable size of @buf (excluding end marker)
* @o: Option number
* @offset: Current offset within @buf, updated on insertion
- *
- * Return: false if @buf has space to write the option, true otherwise
*/
-static bool fill_one(uint8_t *buf, size_t size, int o, int *offset)
+static void fill_one(uint8_t *buf, size_t size, int o, int *offset)
{
size_t slen = opts[o].slen;
/* If we don't have space to write the option, then just skip */
if (*offset + 2 /* code and length of option */ + slen > size)
- return true;
+ return;
buf[*offset] = o;
buf[*offset + 1] = slen;
@@ -439,7 +437,6 @@ static bool fill_one(uint8_t *buf, size_t size, int o, int *offset)
opts[o].sent = 1;
*offset += slen;
- return false;
}
/**
@@ -459,24 +456,18 @@ static int fill(struct msg *m)
* option 53 at the beginning of the list.
* Put it there explicitly, unless requested via option 55.
*/
- if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) {
- if (fill_one(m->o, OPT_MAX, 53, &offset))
- debug("DHCP: skipping option 53");
- }
+ if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen))
+ fill_one(m->o, OPT_MAX, 53, &offset);
for (i = 0; i < opts[55].clen; i++) {
o = opts[55].c[i];
- if (opts[o].conf != OPT_UNSET) {
- if (fill_one(m->o, OPT_MAX, o, &offset))
- debug("DHCP: skipping option %i", o);
- }
+ if (opts[o].conf != OPT_UNSET)
+ fill_one(m->o, OPT_MAX, o, &offset);
}
for (o = 0; o < 255; o++) {
- if (opts[o].conf != OPT_UNSET && !opts[o].sent) {
- if (fill_one(m->o, OPT_MAX, o, &offset))
- debug("DHCP: skipping option %i", o);
- }
+ if (opts[o].conf != OPT_UNSET && !opts[o].sent)
+ fill_one(m->o, OPT_MAX, o, &offset);
}
m->o[offset++] = 255;
--
2.55.0