[Skiboot] [PATCH] libflash: move ffs_flash_read into libflash

[Joel Stanley]

On Tue, Feb 24, 2015 at 3:31 PM, Jeremy Kerr <jk at ozlabs.org> wrote:
> We have ffs_flash_read to do optionally-ecc-ed reads of flash data.
> However, this isn't really related to the ffs partitioning.
>
> This change moves ffs_flash_read into libflash.c, named
> flash_read_corrected. The function itself isn't changed.

See below; I think the existing function has a leak. Aside from that I
agree with the patch.

>
> Signed-off-by: Jeremy Kerr <jk at ozlabs.org>

Reviewed-by: Joel Stanley <joel at jms.id.au>

> diff --git a/libflash/libflash.c b/libflash/libflash.c
> index 5badbff..31a347b 100644
> --- a/libflash/libflash.c
> +++ b/libflash/libflash.c

> +int flash_read_corrected(struct flash_chip *c, uint32_t pos, void *buf,
> +               uint32_t len, bool ecc)
> +{
> +       uint64_t *bufecc;
> +       uint32_t copylen;
> +       int rc;
> +       uint8_t ret;
> +
> +       if (!ecc)
> +               return flash_read(c, pos, buf, len);
> +
> +       /* Copy the buffer in chunks */
> +       bufecc = malloc(ECC_BUFFER_SIZE(COPY_BUFFER_LENGTH));

It looks like we leak bufecc in the non-error path.

> +       if (!bufecc)
> +               return FLASH_ERR_MALLOC_FAILED;
> +
> +       while (len > 0) {
> +               /* What's left to copy? */
> +               copylen = MIN(len, COPY_BUFFER_LENGTH);
> +
> +               /* Read ECCed data from flash */
> +               rc = flash_read(c, pos, bufecc, ECC_BUFFER_SIZE(copylen));
> +               if (rc)
> +                       goto err;
> +
> +               /* Extract data from ECCed data */
> +               ret = eccmemcpy(buf, bufecc, copylen);
> +               if (ret == UE) {
> +                       rc = FLASH_ERR_ECC_INVALID;
> +                       goto err;
> +               }
> +
> +               /* Update for next copy */
> +               len -= copylen;
> +               buf = (uint8_t *)buf + copylen;
> +               pos += ECC_BUFFER_SIZE(copylen);
> +       }
> +
> +       return 0;
> +
> +err:
> +       free(bufecc);
> +       return rc;
> +}