Hi Urja, On Sun, 19 May 2019 at 11:04, Urja Rannikko <urja...@gmail.com> wrote: > > Hi, > > On Sat, May 18, 2019 at 6:05 PM Simon Glass <s...@chromium.org> wrote: > > > > In general we should avoid calling malloc() and free() repeatedly in > > U-Boot lest we turn it into tianocore. In SPL this can make SPI flash > > unusable since free() is often a nop and allocation space is limited. > > > > In any case, these seems no need for malloc() since the number of bytes > > is very small, perhaps less than 8. > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > Fixes: d13f5b254a (spi: Extend the core to ease integration of SPI > > memory controllers) > > > > --- > > > > drivers/spi/spi-mem.c | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c > > index b86eee75bcb..7aabebeff5f 100644 > > --- a/drivers/spi/spi-mem.c > > +++ b/drivers/spi/spi-mem.c > > @@ -201,7 +201,6 @@ int spi_mem_exec_op(struct spi_slave *slave, const > > struct spi_mem_op *op) > > unsigned int pos = 0; > > const u8 *tx_buf = NULL; > > u8 *rx_buf = NULL; > > - u8 *op_buf; > > int op_len; > > u32 flag; > > int ret; > > @@ -338,7 +337,17 @@ int spi_mem_exec_op(struct spi_slave *slave, const > > struct spi_mem_op *op) > > } > > > > op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + > > op->dummy.nbytes; > > - op_buf = calloc(1, op_len); > > + > > + /* > > + * Avoid using malloc() here so that we can use this code in SPL > > where > > + * simple malloc may be used. That implementation does not allow > > free() > > + * so repeated calls to this code can exhaust the space. > > + * > > + * The value of op_len is small, since it does not include the > > actual > > + * data being sent, only the op-code and address. In fact, it > > should be > > + * possible to just use a small fixed value here instead of op_len. > > + */ > > + u8 op_buf[op_len]; > > I'd say just make this a fixed buffer instead of a VLA - less code > space bloat and potential stack problems in case of nonsensical > inputs. > > As for the size, 8 bytes would be fine and actually leave some margin: > the most i would expect for op_len is 1 + 4 + 1 = 6 bytes (the lowest > would be 1+3+0 and the usual 1+3+1).
Is it possible to add a build-time assert for this somewhere? Is VLA very large array? How large is very large? I think the stack is much better place than a fixed buffer for something only 8 bytes in size. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
