[PATCH v5 7/9] cmd: rng: Use a statically allocated array for random bytes

[Simon Glass]

Hi Sughosh,

On Mon, 14 Mar 2022 at 05:27, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> hi Simon,
>
> On Mon, 14 Mar 2022 at 03:53, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Sun, 13 Mar 2022 at 08:49, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > >
> > > Use a statically allocated buffer on stack instead of using malloc for
> > > reading the random bytes. Using a local array is faster than
> > > allocating heap memory on every initiation of the command.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > ---
> > >
> > > Changes since V4:
> > >
> > > * New patch based on review comments from Simon to not use the malloc
> > >   call
> > >
> > >  cmd/rng.c | 17 +++++++----------
> > >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > It might be easier to put this patch before the other one.
> >
> > >
> > > diff --git a/cmd/rng.c b/cmd/rng.c
> > > index 2ddf27545f..81a23964b8 100644
> > > --- a/cmd/rng.c
> > > +++ b/cmd/rng.c
> > > @@ -14,9 +14,9 @@
> > >  static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > >  {
> > >         size_t n;
> > > -       struct udevice *dev;
> > > -       void *buf;
> > > +       u8 buf[64];
> > >         int devnum;
> > > +       struct udevice *dev;
> > >         int ret = CMD_RET_SUCCESS;
> > >
> > >         switch (argc) {
> > > @@ -41,11 +41,10 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > >                 return CMD_RET_FAILURE;
> > >         }
> > >
> > > -       buf = malloc(n);
> > > -       if (!buf) {
> > > -               printf("Out of memory\n");
> > > -               return CMD_RET_FAILURE;
> > > -       }
> > > +       if (!n)
> > > +               return 0;
> > > +
> > > +       n = min(n, sizeof(buf));
> > >
> > >         if (dm_rng_read(dev, buf, n)) {
> > >                 printf("Reading RNG failed\n");
> >
> > This looks like a Windows-style error. How about adding "(err=%d)",
> > ret to this so we can see the error?
>
> Again, this is not being changed by my patch. This is existing code
> which is not being touched by the patch. These kinds of fixes should
> be taken up separately. Thanks.

Ah yes I keep missing that.

- Simon

[..]