Re: [PATCH 5/7] hfsplus: stop using ioctl_by_bdev

From: Damien Le Moal
Date: Thu Apr 23 2020 - 03:42:31 EST


On 2020/04/23 16:16, Christoph Hellwig wrote:
> Instead just call the CD-ROM layer functionality directly.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/hfsplus/wrapper.c | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c
> index 08c1580bdf7a..61eec628805d 100644
> --- a/fs/hfsplus/wrapper.c
> +++ b/fs/hfsplus/wrapper.c
> @@ -127,31 +127,34 @@ static int hfsplus_read_mdb(void *bufptr, struct hfsplus_wd *wd)
> static int hfsplus_get_last_session(struct super_block *sb,
> sector_t *start, sector_t *size)
> {
> - struct cdrom_multisession ms_info;
> - struct cdrom_tocentry te;
> - int res;
> + struct cdrom_device_info *cdi = disk_to_cdi(sb->s_bdev->bd_disk);
>
> /* default values */
> *start = 0;
> *size = i_size_read(sb->s_bdev->bd_inode) >> 9;
>
> if (HFSPLUS_SB(sb)->session >= 0) {
> + struct cdrom_tocentry te;
> +
> + if (!cdi)
> + return -EINVAL;
> +
> te.cdte_track = HFSPLUS_SB(sb)->session;
> te.cdte_format = CDROM_LBA;
> - res = ioctl_by_bdev(sb->s_bdev,
> - CDROMREADTOCENTRY, (unsigned long)&te);
> - if (!res && (te.cdte_ctrl & CDROM_DATA_TRACK) == 4) {
> - *start = (sector_t)te.cdte_addr.lba << 2;
> - return 0;
> + if (cdrom_read_tocentry(cdi, &te) ||
> + (te.cdte_ctrl & CDROM_DATA_TRACK) != 4) {
> + pr_err("invalid session number or type of track\n");
> + return -EINVAL;
> }
> - pr_err("invalid session number or type of track\n");
> - return -EINVAL;
> + *start = (sector_t)te.cdte_addr.lba << 2;
> + } else if (cdi) {
> + struct cdrom_multisession ms_info;
> +
> + ms_info.addr_format = CDROM_LBA;
> + if (cdrom_multisession(cdi, &ms_info) == 0 && ms_info.xa_flag)
> + *start = (sector_t)ms_info.addr.lba << 2;
> }
> - ms_info.addr_format = CDROM_LBA;
> - res = ioctl_by_bdev(sb->s_bdev, CDROMMULTISESSION,
> - (unsigned long)&ms_info);
> - if (!res && ms_info.xa_flag)
> - *start = (sector_t)ms_info.addr.lba << 2;
> +

White space change here, but I think it's ok. I do like a blank line before
return :)

> return 0;
> }
>
>

Looks OK to me.

Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>


--
Damien Le Moal
Western Digital Research