On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <[email protected]> wrote: > > Hi Alexey, > > On 2026-06-04T15:31:05, Alexey Charkov <[email protected]> wrote: > > pxe_utils: extract per-entry key parsing into parse_label_keys() > > > > Split the body of parse_label() into a standalone parse_label_keys() > > helper that walks key/value lines and populates a pre-existing > > struct pxe_label. parse_label() becomes a thin wrapper that creates > > the label, reads its name, attaches it to the menu, and delegates. > > > > This is a pure refactor: the new helper contains the original loop > > verbatim, with the local variable declarations moved to its scope. > > No call sites or behaviour change. > > > > A subsequent change will export this helper so callers parsing > > formats that lack a 'label' header (notably Boot Loader Specification > > type #2 entries) can populate a label directly from a file body > > without duplicating the parser. > > > > Signed-off-by: Alexey Charkov <[email protected]> > > > > boot/pxe_utils.c | 58 > > ++++++++++++++++++++++++++++++++++++-------------------- > > 1 file changed, 37 insertions(+), 21 deletions(-) > > > diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c > > A subsequent change will export this helper so callers parsing > > formats that lack a 'label' header (notably Boot Loader Specification > > type #2 entries) can populate a label directly from a file body > > without duplicating the parser. > > This should be type #1, not type #2. The cover letter and patch 7 both > make clear that BLS type #2 are EFI binaries (out of scope), and the > new bootmeth parses BLS type #1 .conf entry files.
Yes, it's a typo. There's another one like this in another patch - will fix in v2. > > diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c > > @@ -1288,34 +1288,20 @@ static int parse_label_kernel(char **c, struct > > pxe_label *label) > > +static int parse_label_keys(char **c, struct pxe_menu *cfg, > > + struct pxe_label *label) > > Please can you add a kernel-doc style comment, since patch 4 promotes > this to a public symbol. The file uses /** ... */ for the exported > helpers added in patch 2. Sure. The kernel-doc comment was added in the following commit (straight to the header), but adding it here already won't hurt (apart from a slightly larger diff). Thanks for your review! Best regards, Alexey
