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

27 May 2026

On Tue, May 26, 2026 at 06:01:11PM +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 
Reviewed-by: David Gibson 

One pre-existing oddity noted.  Might be nice to address (as a
separate patch) if you have the chance, but it's not a big deal.
...
---
v2:
  - Renamed parameter cap → size.
---
 dhcp.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/dhcp.c b/dhcp.c
index e1c95ad..e126063 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -279,28 +279,27 @@ 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
Pre-existing wart: It's very unusual for bool-valued functions to
report failure with 'true' even though though it *is* usual for
int-valued functions to report failure with negative =~ non-zero =~
"trueish" values.
...
*/
-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);
opts[o].sent = 1;
  *offset += slen;
@@ -325,19 +324,19 @@ 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))
+		if (fill_one(m->o, OPT_MAX, 53, &offset))
      	 debug("DHCP: skipping option 53");
for (i = 0; i < opts[55].clen; i++) {
      o = opts[55].c[i];
      if (opts[o].slen != -1)
-			if (fill_one(m, o, &offset))
+			if (fill_one(m->o, OPT_MAX, o, &offset))
      		debug("DHCP: skipping option %i", o);
  }
for (o = 0; o < 255; o++) {
      if (opts[o].slen != -1 && !opts[o].sent)
-			if (fill_one(m, o, &offset))
+			if (fill_one(m->o, OPT_MAX, o, &offset))
      		debug("DHCP: skipping option %i", o);
  }
-- 
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

    