Re: [PATCH 1/2] Add FAT_IOCTL_GET_VOLUME_LABEL in fat_generic_ioctl()

[Pali RohÃr]

On Saturday 06 October 2018 16:33:10 chen.chenchacha wrote:
> On Thu, 2018-10-04 at 19:33 +0200, Pali RohÃr wrote:
> > On Friday 05 October 2018 01:21:00 chenchacha wrote:
> > > Signed-off-by: chenchacha <chen.chenchacha@xxxxxxxxxxx>
> > > ---
> > >  fs/fat/file.c                 | 22 ++++++++++++++++++++++
> > >  include/uapi/linux/msdos_fs.h |  1 +
> > >  2 files changed, 23 insertions(+)
> > > 
> > > diff --git a/fs/fat/file.c b/fs/fat/file.c
> > > index 4724cc9ad650..56db0b5a8df1 100644
> > > --- a/fs/fat/file.c
> > > +++ b/fs/fat/file.c
> > > @@ -121,10 +121,30 @@ static int fat_ioctl_get_volume_id(struct
> > > inode *inode, u32 __user *user_attr)
> > >  	return put_user(sbi->vol_id, user_attr);
> > >  }
> > >  
> > > +static int fat_ioctl_get_volume_label(struct inode *inode, u8
> > > __user *label)
> > > +{
> > > +	struct super_block *sb = inode->i_sb;
> > > +	struct inode *root_inode = d_inode(sb->s_root);
> > > +	struct buffer_head *bh = NULL;
> > > +	struct msdos_dir_entry *de;
> > > +	int err;
> > > +
> > > +	inode_lock_shared(root_inode);
> > > +	err = fat_get_root_entry(root_inode, &bh, &de);
> > > +	if (err == 0) {
> > > +		if (copy_to_user(label, de->name, MSDOS_NAME))
> > 
> > You need to convert entry name from 8.3 format to label in correct
> > encoding specified by codepage mount option. Plus needs to handle
> > leading 0x03 byte.
> > 
> I think it might be better to put the decoder/encoder on the
> application layter.

I do not think so. On all other places in msdos.ko and vfat.ko driver
which communicate with userspace is that decoder/encoder active and user
does not see raw bytes.

It is really bad to mix encodings and API of different calls.

If in mount option I specified that I want to use XYZ encoding, why
driver does not going to respect it?

> Similarly, the handle leading 0x03 byte is in the
> write function. And make read operation pure, just to carry the volume
> label in root directory to the user.
> 
> What do you think, Pali?
> 

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: PGP signature