Re: [PATCH 01/32] block: Provide blkdev_get_handle_* functions
Jan Kara <[email protected]>
| Newsgroups | org.kernel.vger.reiserfs-devel,dev.linux.lists.dm-devel,dev.linux.lists.drbd-dev,net.sourceforge.lists.linux-f2fs-devel,org.infradead.lists.linux-mtd,org.infradead.lists.linux-nvme,org.kernel.vger.linux-bcache,org.kernel.vger.linux-block,org.kernel.vger.linux-btrfs,org.kernel.vger.linux-ext4,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-nfs,org.kernel.vger.linux-nilfs,org.kernel.vger.linux-pm,org.kernel.vger.linux-raid,org.kernel.vger.linux-s390,org.kernel.vger.linux-scsi,org.kernel.vger.linux-xfs,org.kernel.vger.target-devel,org.kvack.linux-mm,org.ozlabs.lists.linux-erofs,org.xenproject.lists.xen-devel |
|---|---|
| Message-ID | <20230705102128.vquve4qencbbn2br@quack3> |
On Tue 04-07-23 10:28:36, Keith Busch wrote:
> On Tue, Jul 04, 2023 at 02:21:28PM +0200, Jan Kara wrote:
> > +struct bdev_handle *blkdev_get_handle_by_dev(dev_t dev, blk_mode_t mode,
> > + void *holder, const struct blk_holder_ops *hops)
> > +{
> > + struct bdev_handle *handle = kmalloc(sizeof(struct bdev_handle),
> > + GFP_KERNEL);
>
> I believe 'sizeof(*handle)' is the preferred style.
OK.
> > + struct block_device *bdev;
> > +
> > + if (!handle)
> > + return ERR_PTR(-ENOMEM);
> > + bdev = blkdev_get_by_dev(dev, mode, holder, hops);
> > + if (IS_ERR(bdev))
> > + return ERR_CAST(bdev);
>
> Need a 'kfree(handle)' before the error return. Or would it be simpler
> to get the bdev first so you can check the mode settings against a
> read-only bdev prior to the kmalloc?
Yeah. Good point with kfree(). I'm not sure calling blkdev_get_by_dev()
first will be "simpler" - then we need blkdev_put() in case of kmalloc()
failure. Thanks for review!
Honza
--
Jan Kara <jack-IBi9RG/[email protected]>
SUSE Labs, CR