Re: [PATCH v4 2/5] block: add partition parser for U-Boot uImage.FIT

Matthew Wilcox
2022-11-09T13:59:01Z

On Tue, Nov 08, 2022 at 11:03:16PM +0000, Daniel Golle wrote:
> + /* map first page */
> + page = read_mapping_page(
> + mapping, fit_start_sector >> (PAGE_SHIFT - SECTOR_SHIFT), NULL);
> +
> + if (IS_ERR(page))
> + return -EFAULT;
> +
> + if (PageError(page))
> + return -EFAULT;

Why are you checking for PageError? You won't ever get a page with an
error back from read_mapping_page(). And you have the real error in
'page', so why return -EFAUlT, which would indicate a problem copying
from the user. Also, this is a great place to use the new folio APIs
instead of the old page APIs. So:

folio = read_mapping_folio(mapping,
fit_start_sector >> PAGE_SECTORS_SHIFT, NULL);
if (IS_ERR(folio))
return PTR_ERR(folio);

> + init_fit = page_address(page);

init_fit = folio_address(folio) +
offset_in_folio(folio, fit_start_sector * SECTOR_SIZE);

> + if (!init_fit) {
> + put_page(page);
> + return -EFAULT;
> + }

page_address() or folio_address() can't ever return NULL, you should
just drop this nonsense check.

... actually, why can't you call read_part_sector() and avoid all of
this?