Messages in this thread | | | Date | Wed, 9 Nov 2022 13:58:29 +0000 | From | Matthew Wilcox <> | Subject | Re: [PATCH v4 2/5] block: add partition parser for U-Boot uImage.FIT |
| |
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?
| |