On Fri, May 21, 2021 at 12:30:45PM +0100, Matthew Wilcox wrote: > On Wed, May 19, 2021 at 11:22:47PM -0700, Chaitanya Kulkarni wrote: > > The helper functions bio_add_XXX_page() returns the length which is > > unsigned int but the return type of those functions is defined > > as int instead of unsigned int. > > I've been thinking about this for a few weeks as part of the folio > patches: > > https://lore.kernel.org/linux-fsdevel/20210505150628.111735-72-willy@xxxxxxxxxxxxx/ > > - len and off are measured in bytes > - neither are permitted to be negative > - for efficiency we only permit them to be up to 4GB > > I therefore believe the correct type for these parameters to be size_t, > and we should range-check them if they're too large. they should > actually always fit within the page that they're associated with, but > people do allocate non-compound pages and i'm not trying to break that > today. > > using size_t makes it clear that these are byte counts, not (eg) sector > counts. i do think it's good to make the return value unsigned so we > don't have people expecting a negative errno on failure. I think the right type is bool. We always return either 0 or the full length we tried to add. Instead of optimizing for a partial add (which only makes sense for bio_add_hw_page anyway), I'd rather make the interface as simple as possible.