On Mon, Oct 16, 2023 at 09:44:50AM +0200, Marco Felsch wrote:
> Hi Sascha,
> 
> On 23-10-16, Marco Felsch wrote:
> > On 23-10-13, Sascha Hauer wrote:
> > > On Tue, Oct 10, 2023 at 04:33:09PM +0200, Marco Felsch wrote:
> > > > The upcoming commit fixes the HAB support for FlexSPI enabled barebox
> > > > images. This commit prepares the source to keep the diff smaller.
> > > > 
> > > > For the upcoming fix we need a 2nd CSF command sequence which is
> > > > basically the same as the first except for the "Blocks = ...." command.
> > > > Therefore we need to handle the blocks command separately which is done
> > > > by this commit. At the moment there is no functional change.
> > > > 
> > > > Signed-off-by: Marco Felsch <[email protected]>
> > > > ---
> > > >  scripts/imx/imx.c | 22 ++++++++++++++++++++--
> > > >  1 file changed, 20 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/scripts/imx/imx.c b/scripts/imx/imx.c
> > > > index 933019eba449..acc8424e547d 100644
> > > > --- a/scripts/imx/imx.c
> > > > +++ b/scripts/imx/imx.c
> > > > @@ -296,6 +296,24 @@ static int hab_add_str(struct config_data *data, 
> > > > const char *str)
> > > >         return 0;
> > > >  }
> > > >  
> > > > +static int hab_add_barebox_blocks(struct config_data *data,
> > > > +                                 const char *csf_str,
> > > > +                                 const char *flexspi_csf_str)
> > > > +{
> > > > +       int len = strlen(csf_str);
> > > > +
> > > > +       if (data->csf_space < len)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       strcat(data->csf, csf_str);
> > > > +       if (flexspi_csf_str)
> > > > +               strcat(data->flexspi_csf, flexspi_csf_str);
> > > 
> > > Do we need a space check here as well?
> > 
> > To be 100% accurate yes since the strings can have different lengths
> > albeit the difference would be <5 chars. I will rework that, thanks for
> > the review.
> 
> While integrating your review feedback I noticed that my comment was
> wrong. Both strings using the same size limited format string:
> 
>    "Blocks = 0x%08x 0x%08x 0x%08x \"%s\""
> 
> with "%s" always point to data->outfile. So both stings do not differ
> and checking csf_str in enough.

Indeed, so the current way is safe.

However, I wonder if we should rather add a little helper like:

static void *strcata(char *str, const char *add)
{
        str = realloc(str, (str ? strlen(str) : 0) + strlen(add) + 1);
        if (!str)
                return NULL;

        strcat(str, add);

        return str;
}

and get rid oof the fixed size allocation altogether.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Reply via email to