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=202602 header.b=T/o+a4F2; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id AF5275A0269 for ; Wed, 27 May 2026 05:46:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1779853588; bh=pqu1Dni/W11NOl/miN5+R+C3B34BWNlaPUY5s1/RLFo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=T/o+a4F2brPga5YSZ3oMz2MnVajxybFiaA2gJ9XOjOyP0TWOXJ1GQdkP4nyWiJ0ie BaR4n3Edyc0xP0f1biinPUPRqopGxCGoCSnsxhRoBOb4OaB3wIoh9megNpmyNlJqqU xKk08E0CUYzVMTRePVCeAKdglkxZxXJqE1jTaZeQBRXExbD2jQ2yysdVIrGPgZBqAZ e/reaXBZZU9WkKHex2t1ZaAecpo+TBD/2u1GbvlenPJPws009hKcUN2ZxFdUTbqMqZ 0inFH9huYnlbOFENohgA1JGCFOjzzanY9hNs/f5VKq6FHXV6lH+OoQJEpLCsuo1Q1p Kc+NVYEiNoIiQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gQFsc6rY1z4w9R; Wed, 27 May 2026 13:46:28 +1000 (AEST) Date: Wed, 27 May 2026 13:46:12 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH v2 4/6] dhcp: Refactor fill_one() to operate on a generic buffer Message-ID: References: <20260526123115.1226166-1-anskuma@redhat.com> <20260526123115.1226166-5-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="r79p1HzLpfX45Shs" Content-Disposition: inline In-Reply-To: <20260526123115.1226166-5-anskuma@redhat.com> Message-ID-Hash: Z6HHCBY4XNLRBGZHJYRE76OFB3G5YLZE X-Message-ID-Hash: Z6HHCBY4XNLRBGZHJYRE76OFB3G5YLZE 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: --r79p1HzLpfX45Shs Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > Link: https://bugs.passt.top/show_bug.cgi?id=3D192 > 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 =E2=86=92 size. > --- > dhcp.c | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) >=20 > 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__)); > =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 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 =3D~ non-zero =3D~ "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 =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); > =20 > opts[o].sent =3D 1; > *offset +=3D 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"); > =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)) > + if (fill_one(m->o, OPT_MAX, o, &offset)) > debug("DHCP: skipping option %i", o); > } > =20 > for (o =3D 0; o < 255; o++) { > if (opts[o].slen !=3D -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); > } > =20 > --=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 --r79p1HzLpfX45Shs Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmoWaQMACgkQzQJF27ox 2GfqqA/+INQ0FumB9wXVSs780APwOMvgQpPJANpFMkEzhK0d4/9rZiNbZAIGl33O nJxBGjdHwVOYNszoJgjaStNob6ushzjUqPaXdkNgttxff+Wn7YjwfSOtnSckrAh3 3N0UorE4N8xOtXs7JdGuODhQOYZjuw1nGLvJmtwV2wJ6wWdhMgFQeevoXEq8xoQf V3sqErvEk1O8kSmESS87FJs9ieDmJ55FppOmLM/a5YH3gzRBqZWfgQ2dttVo48oF iSZ0je7IF25LLJRUgQV3k217s3z7Tbbr3wKtQI56IdCVlO3WHZa44EV7S4d+xDHX M7lHCVi9NXOUL9DI3GKrO125GlsOz7Zaasg6whMiFmte1bTZKD4OHrAu6W6F/w74 sIeHDFNIvmZHg7qXZE3Ju4Ir1IjODWtJBDEC67aqR7zy/spnj5yTXlmRDuFvw9ih 3dGD//xyGa535Lt7q6aLBaGPVBqZOxtXOpSSvaa0wPxn4qkS+CVYqOVR1j3z3E6R y+bDD2rfKtAox+HKrbUX/VbE+vUbwQyrLIyFcHKYX3xXuQJ7o/3q2F8Jf6++SpUT AL1lgHt+aIYLNCjfwBsH8QMl9mPg0t3yc0PfPA3hj7/fF2sWnPpnVEXBsRcC+mim Ewa2vPCyVc6erJUYqC9kHYxWC7MYBOGwb4W1Vqf3i+mikdp0cpo= =gvw9 -----END PGP SIGNATURE----- --r79p1HzLpfX45Shs--