Hi Mattijs,
On Tuesday, October 13th, 2026 at 4:20 PM, Mattijs Korpershoek <[email protected]> wrote: > Hi Petr, > > Thank you for the patch. > > On Sun, Oct 04, 2026 at 22:48, Petr Hodina via B4 Relay > <[email protected]> wrote: > > > From: Petr Hodina <[email protected]> > > > > As dfu_alt_info is read via env_get() it needs to expand the ${var} > > references using cli_simple_process_macros. > > > > Signed-off-by: Petr Hodina <[email protected]> > > --- > > drivers/dfu/dfu.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c > > index 90d8b45ce8a..fa074db242c 100644 > > --- a/drivers/dfu/dfu.c > > +++ b/drivers/dfu/dfu.c > > @@ -6,6 +6,7 @@ > > * author: Lukasz Majewski <[email protected]> > > */ > > > > +#include <cli.h> > > #include <env.h> > > #include <errno.h> > > #include <log.h> > > @@ -178,7 +179,20 @@ int dfu_init_env_entities(char *interface, char > > *devstr) > > return -EINVAL; > > } > > > > - env_bkp = strdup(str_env); > > + /* > > + * Expand the dfu_alt_info here as it's read via env_get() which > > + * does not expand references the way hush does for a command line > > + */ > > + env_bkp = malloc(strlen(str_env) * 2 + 1); > > Can we explain why we picked strlen(str_env) * 2 + 1? > > Is it an arbitrary choice or is there any justification? > It's just good enough solution to work the problem of flashing the pinephone pro and skipping the USB3 phy enumeration issue for the maskROM and rkdeveloptool commands :) Then again you are right here to ask about the implementation. Looked around and there are two more consumers in 'boot/pxe_utils.c' and 'boot/bootm.c'. My take on this expansion size problem would be to add a helper function that runs the expansion first just counting the size needed. Then allocate the buffer. Say implemented as such: - env_bkp = malloc(strlen(str_env) * 2 + 1); + size = cli_simple_macros_size(str_env); + env_bkp = malloc(size); Alternatively we can preallocate some buffer size and attempt to increase it in a loop (bounded to some max size). What do you think? I'd later also send another patch series that would fix the expansion in the 'boot/pxe_utils.c' as it's fixed to CONFIG_SYS_CBSIZE and does not check the return value. Matching the implementation we deem as a correct one. The reason is we'd like to use the PXE to boot the CI images later as alternative means to the DFU. ''' if (label->append) { char bootargs[CONFIG_SYS_CBSIZE]; cli_simple_process_macros(label->append, bootargs, sizeof(bootargs)); env_set("bootargs", bootargs); } '''
