[PATCH v2] cmd: setexpr: fix no matching string in gsub return empty value

[Francesco Dolcini]

+Tom

Hello Massimiliano,

On Tue, Sep 03, 2024 at 01:06:21PM +0200, Michal Simek wrote:
> HI,
> 
> čt 8. 2. 2024 v 16:00 odesílatel Massimiliano Minella
> <massimiliano.minella at gmail.com> napsal:
> >
> > From: Massimiliano Minella <massimiliano.minella at se.com>
> >
> > In gsub, when the destination string is empty, the string 't' is
> > provided and the regular expression doesn't match, then the final result
> > is an empty string.
> >
> > Example:
> >
> > => echo ${foo}
> >
> > => setenv foo
> > => setexpr foo gsub e a bar
> > => echo ${foo}
> >
> > =>
> >
> > The variable ${foo} should contain "bar" and the lack of match shouldn't
> > be considered an error.
> >
> > This patch fixes the erroneous behavior by removing the return
> > statement and breaking out of the loop in case of lack of match.
> >
> > Also add a test for the no match case.
> >
> > Signed-off-by: Massimiliano Minella <massimiliano.minella at se.com>
> > ---
> > Changes in V2:
> >  - update documentation to describe the behavior
> >
> >  cmd/setexpr.c             |  9 ++++-----
> >  doc/usage/cmd/setexpr.rst |  1 +
> >  test/cmd/setexpr.c        | 10 ++++++++++
> >  3 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/cmd/setexpr.c b/cmd/setexpr.c
> > index 233471f6cb..ab76824a32 100644
> > --- a/cmd/setexpr.c
> > +++ b/cmd/setexpr.c
> > @@ -216,14 +216,12 @@ int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
> >                 if (res == 0) {
> >                         if (loop == 0) {
> >                                 debug("%s: No match\n", data);
> > -                               return 1;
> 
> This patch actually changed the return value from command.
> In board/xilinx/zynqmp/zynqmp_kria.env we have
> 
> bootcmd=setenv model $board_name && if setexpr model gsub
> .*$k24_starter* $k24_starter || setexpr model gsub .*$k26_starter*
> $k26_starter; then run som_cc_boot; else run som_mmc_boot; run
> som_cc_boot; fi
> 
> and this patch actually breaked it because we rely on return value.
> Changing return value is not described and I want to know if this
> patch should be fixed
> or we should update our commands to match new return value.

Massimilano, any comment on this?

This change broke also our use case - our board is no longer booting
with v2024.10 U-Boot. I do not think we should change something like
that without a clear reason and I was not able to understand it from the
commit message.

Francesco