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