On 23-10-16, Sascha Hauer wrote:
> 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 <m.felsch@xxxxxxxxxxxxxx>
> > > > > ---
> > > > > 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.
FTR: I followed your suggestion, please see the v2.
Regards,
Marco
