Re: [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer

On Wed, Jun 17, 2026 at 06:52:35PM +0530, Anshu Kumari wrote:

Change fill_one() to accept a buffer pointer and capacity instead of a struct msg pointer. This is a pure refactor with no behavior change, preparing for option overload support where fill_one() will also write into the file and sname fields.

Link: https://bugs.passt.top/show_bug.cgi?id=192 Signed-off-by: Anshu Kumari --- v4: - Unchanged from v3.

Uh.. this doesn't appear to be true...

v3: - Restored removed comments: "If we don't have space to write the option, then just skip" and "Move to option".

v2: - Renamed parameter cap → size. --- dhcp.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/dhcp.c b/dhcp.c index 1ff8cba..8e3ffd6 100644 --- a/dhcp.c +++ b/dhcp.c @@ -131,28 +131,29 @@ struct msg { } __attribute__((__packed__));

/** - * fill_one() - Fill a single option in message - * @m: Message to fill + * fill_one() - Fill a single option into a buffer + * @buf: Buffer to write option + * @size: Usable size of @buf (excluding end marker) * @o: Option number - * @offset: Current offset within options field, updated on insertion + * @offset: Current offset within @buf, updated on insertion * - * Return: false if m has space to write the option, true otherwise + * Return: false if @buf has space to write the option, true otherwise */ -static bool fill_one(struct msg *m, int o, int *offset) +static bool 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 > OPT_MAX) + if (*offset + 2 + slen > size) return true;

- m->o[*offset] = o; - m->o[*offset + 1] = slen; + buf[*offset] = o; + buf[*offset + 1] = slen;

/* Move to option */ *offset += 2;

- memcpy(&m->o[*offset], opts[o].s, slen); + memcpy(&buf[*offset], opts[o].s, slen);

..LGTM so far, but..

opts[o].sent = 1; *offset += slen; @@ -177,20 +178,17 @@ static int fill(struct msg *m) * 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, 53, &offset)) - debug("DHCP: skipping option 53"); + fill_one(m->o, OPT_MAX, 53, &offset);

..you've removed all the debug messages on failed inserts. Maybe there's a reason for that, but it's not mentioned in the commit message, and is new in v4.

for (i = 0; i < opts[55].clen; i++) { o = opts[55].c[i]; if (opts[o].slen != -1) - if (fill_one(m, o, &offset)) - debug("DHCP: skipping option %i", o); + fill_one(m->o, OPT_MAX, o, &offset); }

for (o = 0; o < 255; o++) { if (opts[o].slen != -1 && !opts[o].sent) - if (fill_one(m, o, &offset)) - debug("DHCP: skipping option %i", o); + fill_one(m->o, OPT_MAX, o, &offset); }

m->o[offset++] = 255; -- 2.54.0

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson