Cleanup unload_nls() calls
Subsystems (7)
cover Cleanup unload_nls() calls
While looking through the BKL call sites in fs/* I noticed that most callsites of unload_nls() do
if (nls) unload_nls(nls);
The patch series moves the NULL pointer check into unload_nls() and cleans up the various users.
Thanks,
tglx
---
befs/linuxvfs.c | 7 +------ cifs/cifsfs.c | 3 +-- fat/inode.c | 16 ++++------------ hfs/mdb.c | 6 ++---- hfsplus/super.c | 6 ++---- isofs/inode.c | 8 ++------ jfs/super.c | 9 +++------ ncpfs/inode.c | 12 ++---------- ncpfs/ioctl.c | 6 ++---- nls/nls_base.c | 3 ++- ntfs/super.c | 10 ++++------ smbfs/inode.c | 10 ++-------- 12 files changed, 27 insertions(+), 69 deletions(-)
PATCH 1/11 fs: Make unload_nls() NULL pointer safe
Most callsites of unload_nls() do
if (nls) unload_nls(nls);
Check the pointer inside unload_nls() like we do in kfree() so we can simplify the call sites.
--- fs/nls/nls_base.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
fs/nls/nls_base.c +2 -1
@@ -270,7 +270,8 @@ struct nls_table *load_nls(char *charsetvoid unload_nls (struct nls_table *nls) { -module_put (nls->owner); +if (nls) +module_put (nls->owner); }static const wchar_t charset2uni[256 ] = {
PATCH 2/11 fat: cleanup fat_put_super()
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer checks and the NULLification of the pointers as the data structure which contains the pointers is kfree'd right away.
--- fs/fat/inode.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
fs/fat/inode.c +4 -12
@@ -470,19 +470,11 @@ static void fat_put_super(struct super_biput (sbi->fat_inode); -if (sbi->nls_disk) { -unload_nls (sbi->nls_disk); - sbi->nls_disk =NULL ; - sbi->options.codepage = fat_default_codepage; - } -if (sbi->nls_io) { -unload_nls (sbi->nls_io); - sbi->nls_io =NULL ; - } -if (sbi->options.iocharset != fat_default_iocharset) { +unload_nls (sbi->nls_disk); +unload_nls (sbi->nls_io); + +if (sbi->options.iocharset != fat_default_iocharset)kfree (sbi->options.iocharset); - sbi->options.iocharset = fat_default_iocharset; - } sb->s_fs_info =NULL ;kfree (sbi);
OGAWA Hirofumi
maintainerRe: [patch 02/11] fat: cleanup fat_put_super()
Quoted 20 lines · Thomas Gleixner
unload_nls()can be called with a NULL pointer now. Remove the pointer checks and the NULLification of the pointers as the data structure which contains the pointers is kfree'd right away.Signed-off-by Thomas Gleixner <[email protected]>Cc OGAWA Hirofumi <[email protected]>--- fs/fat/inode.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)Index:
linux-2.6-tip/fs/fat/inode.c
===================================================================fs/fat/inode.c +0
-2@@ -470,19 +470,11 @@ static void fat_put_super(struct super_biput (sbi->fat_inode); -if (sbi->nls_disk) { -unload_nls (sbi->nls_disk);
fs/fat/inode.c static void fat_put_super(struct super_b · line 474
Quoted 10 lines
fs/fat/inode.c +2
-8- sbi->nls_disk =NULL ; - sbi->options.codepage = fat_default_codepage; - } -if (sbi->nls_io) { -unload_nls (sbi->nls_io); - sbi->nls_io =NULL ; - } -if (sbi->options.iocharset != fat_default_iocharset) { +unload_nls (sbi->nls_disk); +unload_nls (sbi->nls_io);
I don't object to this, however, personally I'd prefer to check NULL explicitly, including brelse().
Thanks.
Signature · 1 line
OGAWA Hirofumi <[email protected]>
PATCH 3/11 befs: cleanup befs_put_super()
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer checks and the NULLification of the pointer as the data structure which contains the pointer is kfree'd right away.
--- fs/befs/linuxvfs.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
fs/befs/linuxvfs.c +1 -6
@@ -737,12 +737,7 @@ befs_put_super(struct super_block *sb) {kfree (BEFS_SB (sb)->mount_opts.iocharset);BEFS_SB (sb)->mount_opts.iocharset =NULL ; - -if (BEFS_SB (sb)->nls) { -unload_nls (BEFS_SB (sb)->nls); -BEFS_SB (sb)->nls =NULL ; - } - +unload_nls (BEFS_SB (sb)->nls);kfree (sb->s_fs_info); sb->s_fs_info =NULL ; }
PATCH 4/11 cifs: cleanup unload_nls() call
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer check.
--- fs/cifs/cifsfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
fs/cifs/cifsfs.c +1 -2
@@ -185,8 +185,7 @@ out_mount_failed: cifs_sb->mountdata = NULL; }#endif -if (cifs_sb->local_nls) -unload_nls (cifs_sb->local_nls); +unload_nls (cifs_sb->local_nls);kfree (cifs_sb); }return rc;
PATCH 5/11 hfs: clenaup unload_nls() calls
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer checks.
--- fs/hfs/mdb.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
fs/hfs/mdb.c +2 -4
@@ -344,10 +344,8 @@ void hfs_mdb_put(struct super_block *sb)brelse (HFS_SB (sb)->mdb_bh);brelse (HFS_SB (sb)->alt_mdb_bh); -if (HFS_SB (sb)->nls_io) -unload_nls (HFS_SB (sb)->nls_io); -if (HFS_SB (sb)->nls_disk) -unload_nls (HFS_SB (sb)->nls_disk); +unload_nls (HFS_SB (sb)->nls_io); +unload_nls (HFS_SB (sb)->nls_disk);free_pages ((unsigned long )HFS_SB (sb)->bitmap,PAGE_SIZE <8192 ?1 :0 );kfree (HFS_SB (sb));
PATCH 6/11 hfsplus: cleanup unload_nls() calls
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer checks.
--- fs/hfsplus/super.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
fs/hfsplus/super.c +2 -4
@@ -229,8 +229,7 @@ static void hfsplus_put_super(struct supiput (HFSPLUS_SB (sb).alloc_file);iput (HFSPLUS_SB (sb).hidden_dir);brelse (HFSPLUS_SB (sb).s_vhbh); -if (HFSPLUS_SB (sb).nls) -unload_nls (HFSPLUS_SB (sb).nls); +unload_nls (HFSPLUS_SB (sb).nls);kfree (sb->s_fs_info); sb->s_fs_info =NULL ;
@@ -464,8 +463,7 @@ out: cleanup:hfsplus_put_super (sb); -if (nls) -unload_nls (nls); +unload_nls (nls);return err; }
PATCH 7/11 isofs: cleanup unload_nls() calls
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer checks and the NULLification of the pointers as the data structure which contains the pointers is kfree'd right away.
--- fs/isofs/inode.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
fs/isofs/inode.c +2 -6
@@ -46,10 +46,7 @@ static void isofs_put_super(struct super#ifdef CONFIG_JOLIET lock_kernel (); -if (sbi->s_nls_iocharset) { -unload_nls (sbi->s_nls_iocharset); - sbi->s_nls_iocharset =NULL ; - } +unload_nls (sbi->s_nls_iocharset);unlock_kernel ();#endif
@@ -912,8 +909,7 @@ out_no_root:printk (KERN_WARNING "%s: get root inode failed\n" , __func__); out_no_inode:#ifdef CONFIG_JOLIET -if (sbi->s_nls_iocharset) -unload_nls (sbi->s_nls_iocharset); +unload_nls (sbi->s_nls_iocharset);#endif goto out_freesbi; out_no_read:
PATCH 8/11 jfs: cleanup unload_nls() calls
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer check and the NULLification of the pointer as the data structure which contains the pointer is kfree'd right away.
--- fs/jfs/super.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
fs/jfs/super.c +3 -6
@@ -178,13 +178,11 @@ static void jfs_put_super(struct super_b rc =jfs_umount (sb);if (rc)jfs_err ("jfs_umount failed with return code %d" , rc); -if (sbi->nls_tab) -unload_nls (sbi->nls_tab); - sbi->nls_tab =NULL ; + +unload_nls (sbi->nls_tab);truncate_inode_pages (sbi->direct_inode->i_mapping,0 );iput (sbi->direct_inode); - sbi->direct_inode =NULL ;kfree (sbi);
@@ -347,8 +345,7 @@ static int parse_options(char *options,if (nls_map != (void *)-1 ) {/* Discard old (if remount) */ -if (sbi->nls_tab) -unload_nls (sbi->nls_tab); +unload_nls (sbi->nls_tab); sbi->nls_tab = nls_map; }return 1 ;
Re: [patch 08/11] jfs: cleanup unload_nls() calls
Quoted 5 lines · Thomas Gleixner
unload_nls()can be called with a NULL pointer now. Remove the pointer check and the NULLification of the pointer as the data structure which contains the pointer is kfree'd right away.Signed-off-by Thomas Gleixner <[email protected]>
Quoted 3 lines
--- fs/jfs/super.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
Signature · 2 lines
David Kleikamp IBM Linux Technology Center
PATCH 9/11 ncpfs: cleanup unload_nls() calls
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer checks and the NULLification of the pointers as the data structure which contains the pointers is kfree'd right away.
--- fs/ncpfs/inode.c | 12 ++---------- fs/ncpfs/ioctl.c | 6 ++---- 2 files changed, 4 insertions(+), 14 deletions(-)
fs/ncpfs/inode.c +2 -10
@@ -746,16 +746,8 @@ static void ncp_put_super(struct super_b#ifdef CONFIG_NCPFS_NLS /* unload the NLS charsets */ -if (server->nls_vol) - { -unload_nls (server->nls_vol); - server->nls_vol =NULL ; - } -if (server->nls_io) - { -unload_nls (server->nls_io); - server->nls_io =NULL ; - } +unload_nls (server->nls_vol); +unload_nls (server->nls_io);#endif /* CONFIG_NCPFS_NLS */ if (server->info_filp)
fs/ncpfs/ioctl.c +2 -4
@@ -223,10 +223,8 @@ ncp_set_charsets(struct ncp_server* serv oldset_io = server->nls_io; server->nls_io = iocharset; -if (oldset_cp) -unload_nls (oldset_cp); -if (oldset_io) -unload_nls (oldset_io); +unload_nls (oldset_cp); +unload_nls (oldset_io);return 0 ; }
PATCH 10/11 ntfs: cleanup unload_nls() calls
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer checks and the NULLification of the pointers as the data structure which contains the pointers is kfree'd right away.
--- fs/ntfs/super.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
fs/ntfs/super.c +4 -6
@@ -201,8 +201,7 @@ use_utf8: v, old_nls->charset); nls_map = old_nls; }else /* nls_map */ { -if (old_nls) -unload_nls (old_nls); +unload_nls (old_nls); } }else if (!strcmp (p,"utf8" )) {bool val = false;
@@ -2427,10 +2426,9 @@ static void ntfs_put_super(struct super_ntfs_free (vol->upcase); vol->upcase =NULL ; } -if (vol->nls_map) { -unload_nls (vol->nls_map); - vol->nls_map =NULL ; - } + +unload_nls (vol->nls_map); + sb->s_fs_info =NULL ;kfree (vol);
PATCH 11/11 smbfs: cleanup unload_nls() calls
In reply to Thomas Gleixnerunload_nls() can be called with a NULL pointer now. Remove the pointer checks and the NULLification of the pointers as the data structure which contains the pointers is kfree'd right away.
--- fs/smbfs/inode.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
fs/smbfs/inode.c +2 -8
@@ -459,14 +459,8 @@ smb_show_options(struct seq_file *s, strstatic void smb_unload_nls (struct smb_sb_info *server) { -if (server->remote_nls) { -unload_nls (server->remote_nls); - server->remote_nls =NULL ; - } -if (server->local_nls) { -unload_nls (server->local_nls); - server->local_nls =NULL ; - } +unload_nls (server->remote_nls); +unload_nls (server->local_nls); } static void
Quoted 8 lines · Thomas Gleixner
While looking through the BKL call sites in
fs/*I noticed that most callsites ofunload_nls()doif (nls) unload_nls(nls);The patch series moves the NULL pointer check into
unload_nls()and cleans up the various users.
This looks all good to me. But I'd just make it a single patch..
Quoted 11 lines · Christoph Hellwig
Quoted 8 lines · Thomas Gleixner
While looking through the BKL call sites in
fs/*I noticed that most callsites ofunload_nls()doif (nls) unload_nls(nls);The patch series moves the NULL pointer check into
unload_nls()and cleans up the various users.This looks all good to me. But I'd just make it a single patch..
Sure, should go via Al, right ?
Thanks,
tglx