From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202606 header.b=UKBC2EV0; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 2F2655A026D for ; Fri, 19 Jun 2026 05:54:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1781841241; bh=0diILe4eLqx3f7Z1KjZM7MTtyrHZimac8yIehVAtngY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UKBC2EV0mXF64hO/meDHUX0n50XszNc4wQueQsFk6gxMGvvRGjGFhOJbggSj3rDS7 mWh2/U4K5OCHl3kJNog5grVzqNcymAIarhR4qEEKvb9exzdvsnDxDurlcWxqTvaTyj obwdFVtsYEnba0kdpKwmBK5XlKD6sZA/yBy16p9ENMPr8rN8sJpq1vzitfulc5utI6 JEU5OEA02HouJsXk8idQ6OySrAXq4DqwTNdOhrpTwV28NRlpcIkPqNrvxC+lr2Q5gf UeVsuAERm2VRuvopH/fjHpeCIkD/0oayMdwIyZvc77sw90vdCDOOD8nLk7L+K3spDn MIogW2I0/yw/g== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ghNxj6SrBz58sh; Fri, 19 Jun 2026 13:54:01 +1000 (AEST) Date: Fri, 19 Jun 2026 13:26:00 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH v4 1/4] dhcp: Refactor fill_one() to operate on a generic buffer Message-ID: References: <20260617132243.1499556-1-anskuma@redhat.com> <20260617132243.1499556-2-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="OTEoYmzThhMti2sW" Content-Disposition: inline In-Reply-To: <20260617132243.1499556-2-anskuma@redhat.com> Message-ID-Hash: BYEMFMSXLNAYRGUR6RXINAKRKSJE6VRK X-Message-ID-Hash: BYEMFMSXLNAYRGUR6RXINAKRKSJE6VRK X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, sbrivio@redhat.com, jmaloy@redhat.com, lvivier@redhat.com X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --OTEoYmzThhMti2sW Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > Link: https://bugs.passt.top/show_bug.cgi?id=3D192 > Signed-off-by: Anshu Kumari > --- > v4: > - Unchanged from v3. Uh.. this doesn't appear to be true... >=20 > v3: > - Restored removed comments: "If we don't have space to write the > option, then just skip" and "Move to option". >=20 > v2: > - Renamed parameter cap =E2=86=92 size. > --- > dhcp.c | 28 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 15 deletions(-) >=20 > 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__)); > =20 > /** > - * 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 =3D opts[o].slen; > =20 > /* 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; > =20 > - m->o[*offset] =3D o; > - m->o[*offset + 1] =3D slen; > + buf[*offset] =3D o; > + buf[*offset + 1] =3D slen; > =20 > /* Move to option */ > *offset +=3D 2; > =20 > - memcpy(&m->o[*offset], opts[o].s, slen); > + memcpy(&buf[*offset], opts[o].s, slen); =2E.LGTM so far, but.. > =20 > opts[o].sent =3D 1; > *offset +=3D 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); =2E.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. > =20 > for (i =3D 0; i < opts[55].clen; i++) { > o =3D opts[55].c[i]; > if (opts[o].slen !=3D -1) > - if (fill_one(m, o, &offset)) > - debug("DHCP: skipping option %i", o); > + fill_one(m->o, OPT_MAX, o, &offset); > } > =20 > for (o =3D 0; o < 255; o++) { > if (opts[o].slen !=3D -1 && !opts[o].sent) > - if (fill_one(m, o, &offset)) > - debug("DHCP: skipping option %i", o); > + fill_one(m->o, OPT_MAX, o, &offset); > } > =20 > m->o[offset++] =3D 255; > --=20 > 2.54.0 >=20 --=20 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 --OTEoYmzThhMti2sW Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmo0troACgkQzQJF27ox 2GeONxAApqcHw3TqaM5YI0OkULLTZ3OIn+GM5z5o1MBFcOBr9F5LCH5trVk6vyM/ GEjYV0uM2JjrRB9piDjm/9OJQ+kIYN/uNpHO5C8yWcnWqi4z7lm/BmFrPHUBV+dg IiV4pDE+Fgt5iU2xcZtgOG7L2t+cZNTefjNKKIA5rfpcZNvfyeJeD3I6VVOVxMgg jcT/poxKNJblm+ByRT861PeWdoVYofjfhREyZyIcL7vVBUyumjYSV9NVpvpUCw/l ITulZs8beOTBPKvg89Y5FjJRouM9KTVfgScvrU3aE9BDKIr79/Me7xWMl9PVrDKw ihKqjcQy7hP/uviBJLqOmStWZK0M/i7FztY6+YLPCD98iMxSZbIsoeTz8qB1yAHg jaNki3xuGfdpGukRUNuRJDraDj7ygDLSGHNgnFlZaCAENqQY65FW5q2+DuPlqUQz wPbURxfELVomTCpd9DCL2spjEOMDc/rePP7G+CItQrurgNeZnmiaeFApAnAiA6RN xaTQsSFps3t3oSqvQJ1MN66JK2qV+n24MkGtyGtOBNpvFKAXsKNqE3JUOO2nNRG+ epLoTBzZkTQKJsjrJuCiunNDOpBIyfJIQVbztV7yNSIX0ejlBzEYGO9C3w+2EwS0 B25aD4ohh+PpT8NPYlIZ0omLUduG2aAPO8xMtjigjZiasIZ1AKI= =hIie -----END PGP SIGNATURE----- --OTEoYmzThhMti2sW--