Re: [TRIVIAL] Error checks omitted in init_tmpfs() in mm/tiny-shmem.c

[Matt Mackall <mpm@selenic.com>]

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.

Hmm. Did you actually encounter this?

I'd rather use BUG_ON. Passing up errors is only useful when the code
above can and will do something useful with the information. 

Here, we're talking about a built-in getting initialized at boot that
can quietly fail and the upper layer will simply shrug and go on,
leaving the system in a possibly useless state. Which is worse than
not attempting error handling at all, because we've added complexity
for no gain.

And what could the higher level, which is simply looping through init
functions, do to handle the error? Retry? Print a warning? Better to
stop everything outright when we encounter a problem we expect should
never happen so it doesn't go by undiagnosed.

> --- 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 00:13:10.532652000 -0500
> @@ -31,12 +31,27 @@
>  
>  static int __init init_tmpfs(void)
>  {
> -	register_filesystem(&tmpfs_fs_type);
> +	int error;
> +
> +	error = register_filesystem(&tmpfs_fs_type);
> +	if (error) {
> +		goto out2;
> +	}
> +
>  #ifdef CONFIG_TMPFS
>  	devfs_mk_dir("shm");
>  #endif
>  	shm_mnt = kern_mount(&tmpfs_fs_type);
> +	if (IS_ERR(shm_mnt)) {
> +		error = PTR_ERR(shm_mnt);
> +		goto out1;
> +	}
> +
>  	return 0;
> +out1:
> +	unregister_filesystem(&tmpfs_fs_type);
> +out2:
> +	return error;
>  }
>  module_init(init_tmpfs)
>  


-- 
Mathematics is the supreme nostalgia of our time.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/