Messages in this thread Patch in this message | | | Date | Mon, 24 Oct 2005 19:16:50 -0500 | From | Hareesh Nagarajan <> | Subject | Re: [TRIVIAL] Error checks omitted in init_tmpfs() in mm/tiny-shmem.c |
| |
Matt Mackall wrote: > On Mon, Oct 24, 2005 at 03:56:47AM -0500, Hareesh Nagarajan wrote: >> Matt Mackall wrote: >>> On Mon, Oct 24, 2005 at 12:29:45AM -0500, Hareesh Nagarajan wrote: >>>> The existing code in init_tmpfs() in mm/tiny-shmem.c does not handle the >>>> cases when the calls to register_filesystem() and kern_mount() fail. >>>> This patch adds those checks.
[ Snip ]
> A couple more comments.. > >> Signed-off-by: Hareesh Nagarajan <hnagar2@gmail.com> > >> --- linux-2.6.13.4/mm/tiny-shmem.c 2005-10-10 13:54:29.000000000 -0500 >> +++ linux-2.6.13.4-edit/mm/tiny-shmem.c 2005-10-24 03:43:38.614071000 -0500 >> @@ -31,12 +31,18 @@ >> >> static int __init init_tmpfs(void) >> { >> - register_filesystem(&tmpfs_fs_type); >> + int error; >> + >> + error = register_filesystem(&tmpfs_fs_type); >> + BUG_ON(error); > > Can we just do BUG_ON(register_filesystem() != 0)? > > Strictly speaking, the != 0 is redundant, but as this goes slightly > against the grain of normal usage, it's a good indicator of intent.
It shows intent well. That goes into my book for good programming practices. No more BUG_ON(foo) :)
>> #ifdef CONFIG_TMPFS >> devfs_mk_dir("shm"); >> #endif >> shm_mnt = kern_mount(&tmpfs_fs_type); >> - return 0; >> + BUG_ON(IS_ERR(shm_mnt)); >> + >> + return error; > > We can never return non-zero here. Returning error implies we can, so > it's confusing.
Makes sense again. Patch follows.
Signed-off-by: Hareesh Nagarajan <hnagar2@gmail.com> --- linux-2.6.13.4/mm/tiny-shmem.c 2005-10-10 13:54:29.000000000 -0500 +++ linux-2.6.13.4-edit/mm/tiny-shmem.c 2005-10-24 19:05:28.058897000 -0500 @@ -31,11 +31,14 @@ static int __init init_tmpfs(void) { - register_filesystem(&tmpfs_fs_type); + BUG_ON(register_filesystem(&tmpfs_fs_type) != 0); + #ifdef CONFIG_TMPFS devfs_mk_dir("shm"); #endif shm_mnt = kern_mount(&tmpfs_fs_type); + BUG_ON(IS_ERR(shm_mnt)); + return 0; } module_init(init_tmpfs) | |