Re: [PATCH 1/1] fs/jfs/super.c: convert simple_str to kstr

[Dave Kleikamp <dave.kleikamp@oracle.com>]

Fabian,
I finally got to pushing this and your previous jfs cleanups to
git://github.com/kleikamp/linux-shaggy.git jfs-next

I'll give them a day or two in the next tree, then push them to Linus.

I modified this patch slightly, but the others are unchanged.

Thanks,
Dave

On 05/21/2014 01:29 PM, Fabian Frederick wrote:
> This patch replaces obsolete simple_str functions by kstr
> 
> use kstrtouint for
> -uid_t ( __kernel_uid32_t )
> -gid_t ( __kernel_gid32_t )
> -jfs_sb_info->umask
> -jfs_sb_info->minblks_trim
> (all unsigned int)
> 
> newLVSize is s64 -> use kstrtol
> 
> Current parse_options behaviour stays the same ie it doesn't return kstr
> rc but just 0 if function failed (parse_options callsites
> return -EINVAL when there's anything wrong).
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
> ---
>  fs/jfs/super.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index 97f7fda..fe09e5d 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -273,7 +273,10 @@ static int parse_options(char *options, struct super_block *sb, s64 *newLVSize,
>  		case Opt_resize:
>  		{
>  			char *resize = args[0].from;
> -			*newLVSize = simple_strtoull(resize, &resize, 0);
> +			int rc = kstrtoll(resize, 0, newLVSize);
> +
> +			if (rc)
> +				return 0;

Should goto cleanup here, or it's possible to leak a newly allocated
nls_map.

>  			break;
>  		}
>  		case Opt_resize_nosize:
> @@ -327,7 +330,11 @@ static int parse_options(char *options, struct super_block *sb, s64 *newLVSize,
>  		case Opt_uid:
>  		{
>  			char *uid = args[0].from;
> -			uid_t val = simple_strtoul(uid, &uid, 0);
> +			uid_t val;
> +			int rc = kstrtouint(uid, 0, &val);
> +
> +			if (rc)
> +				return 0;

same here

>  			sbi->uid = make_kuid(current_user_ns(), val);
>  			if (!uid_valid(sbi->uid))
>  				goto cleanup;
> @@ -337,7 +344,11 @@ static int parse_options(char *options, struct super_block *sb, s64 *newLVSize,
>  		case Opt_gid:
>  		{
>  			char *gid = args[0].from;
> -			gid_t val = simple_strtoul(gid, &gid, 0);
> +			gid_t val;
> +			int rc = kstrtouint(gid, 0, &val);
> +
> +			if (rc)
> +				return 0;

same here

>  			sbi->gid = make_kgid(current_user_ns(), val);
>  			if (!gid_valid(sbi->gid))
>  				goto cleanup;
> @@ -347,7 +358,10 @@ static int parse_options(char *options, struct super_block *sb, s64 *newLVSize,
>  		case Opt_umask:
>  		{
>  			char *umask = args[0].from;
> -			sbi->umask = simple_strtoul(umask, &umask, 8);
> +			int rc = kstrtouint(umask, 8, &sbi->umask);
> +
> +			if (rc)
> +				return 0;

same here

>  			if (sbi->umask & ~0777) {
>  				pr_err("JFS: Invalid value of umask\n");
>  				goto cleanup;
> @@ -380,10 +394,13 @@ static int parse_options(char *options, struct super_block *sb, s64 *newLVSize,
>  		{
>  			struct request_queue *q = bdev_get_queue(sb->s_bdev);
>  			char *minblks_trim = args[0].from;
> +			int rc;
>  			if (blk_queue_discard(q)) {
>  				*flag |= JFS_DISCARD;
> -				sbi->minblks_trim = simple_strtoull(
> -					minblks_trim, &minblks_trim, 0);
> +				rc = kstrtouint(minblks_trim, 0,
> +					       &sbi->minblks_trim);
> +				if (rc)
> +					return 0;

and here too

>  			} else {
>  				pr_err("JFS: discard option " \
>  					"not supported on device\n");
>