linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/3] parser: add unsigned int parser
@ 2021-01-28  2:20 bingjingc
  2021-01-28  3:27 ` Randy Dunlap
  2021-01-28  8:32 ` Miklos Szeredi
  0 siblings, 2 replies; 4+ messages in thread
From: bingjingc @ 2021-01-28  2:20 UTC (permalink / raw)
  To: viro, jack, jack, axboe, linux-fsdevel
  Cc: linux-kernel, cccheng, bingjingc, robbieko

From: BingJing Chang <bingjingc@synology.com>

Will be used by fs parsing options

Reviewed-by: Robbie Ko<robbieko@synology.com>
Reviewed-by: Chung-Chiang Cheng <cccheng@synology.com>
Signed-off-by: BingJing Chang <bingjingc@synology.com>
---
 fs/isofs/inode.c       | 16 ++--------------
 fs/udf/super.c         | 16 ++--------------
 include/linux/parser.h |  1 +
 lib/parser.c           | 22 ++++++++++++++++++++++
 4 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index 342ac19..21edc42 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -335,18 +335,6 @@ static const match_table_t tokens = {
 	{Opt_err, NULL}
 };
 
-static int isofs_match_uint(substring_t *s, unsigned int *res)
-{
-	int err = -ENOMEM;
-	char *buf = match_strdup(s);
-
-	if (buf) {
-		err = kstrtouint(buf, 10, res);
-		kfree(buf);
-	}
-	return err;
-}
-
 static int parse_options(char *options, struct iso9660_options *popt)
 {
 	char *p;
@@ -447,7 +435,7 @@ static int parse_options(char *options, struct iso9660_options *popt)
 		case Opt_ignore:
 			break;
 		case Opt_uid:
-			if (isofs_match_uint(&args[0], &uv))
+			if (match_uint(&args[0], &uv))
 				return 0;
 			popt->uid = make_kuid(current_user_ns(), uv);
 			if (!uid_valid(popt->uid))
@@ -455,7 +443,7 @@ static int parse_options(char *options, struct iso9660_options *popt)
 			popt->uid_set = 1;
 			break;
 		case Opt_gid:
-			if (isofs_match_uint(&args[0], &uv))
+			if (match_uint(&args[0], &uv))
 				return 0;
 			popt->gid = make_kgid(current_user_ns(), uv);
 			if (!gid_valid(popt->gid))
diff --git a/fs/udf/super.c b/fs/udf/super.c
index efeac8c..2f83c12 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -454,18 +454,6 @@ static const match_table_t tokens = {
 	{Opt_err,	NULL}
 };
 
-static int udf_match_uint(substring_t *s, unsigned int *res)
-{
-	int err = -ENOMEM;
-	char *buf = match_strdup(s);
-
-	if (buf) {
-		err = kstrtouint(buf, 10, res);
-		kfree(buf);
-	}
-	return err;
-}
-
 static int udf_parse_options(char *options, struct udf_options *uopt,
 			     bool remount)
 {
@@ -521,7 +509,7 @@ static int udf_parse_options(char *options, struct udf_options *uopt,
 			uopt->flags &= ~(1 << UDF_FLAG_USE_SHORT_AD);
 			break;
 		case Opt_gid:
-			if (udf_match_uint(args, &uv))
+			if (match_uint(args, &uv))
 				return 0;
 			uopt->gid = make_kgid(current_user_ns(), uv);
 			if (!gid_valid(uopt->gid))
@@ -529,7 +517,7 @@ static int udf_parse_options(char *options, struct udf_options *uopt,
 			uopt->flags |= (1 << UDF_FLAG_GID_SET);
 			break;
 		case Opt_uid:
-			if (udf_match_uint(args, &uv))
+			if (match_uint(args, &uv))
 				return 0;
 			uopt->uid = make_kuid(current_user_ns(), uv);
 			if (!uid_valid(uopt->uid))
diff --git a/include/linux/parser.h b/include/linux/parser.h
index 89e2b23..dd79f45 100644
--- a/include/linux/parser.h
+++ b/include/linux/parser.h
@@ -29,6 +29,7 @@ typedef struct {
 
 int match_token(char *, const match_table_t table, substring_t args[]);
 int match_int(substring_t *, int *result);
+int match_uint(substring_t *s, unsigned int *result);
 int match_u64(substring_t *, u64 *result);
 int match_octal(substring_t *, int *result);
 int match_hex(substring_t *, int *result);
diff --git a/lib/parser.c b/lib/parser.c
index f5b3e5d..2ec9c4f 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -189,6 +189,28 @@ int match_int(substring_t *s, int *result)
 EXPORT_SYMBOL(match_int);
 
 /**
+ * match_uint: - scan a decimal representation of an integer from a substring_t
+ * @s: substring_t to be scanned
+ * @result: resulting integer on success
+ *
+ * Description: Attempts to parse the &substring_t @s as a decimal integer. On
+ * success, sets @result to the integer represented by the string and returns 0.
+ * Returns -ENOMEM, -EINVAL, or -ERANGE on failure.
+ */
+int match_uint(substring_t *s, unsigned int *result)
+{
+	int err = -ENOMEM;
+	char *buf = match_strdup(s);
+
+	if (buf) {
+		err = kstrtouint(buf, 10, result);
+		kfree(buf);
+	}
+	return err;
+}
+EXPORT_SYMBOL(match_uint);
+
+/**
  * match_u64: - scan a decimal representation of a u64 from
  *                  a substring_t
  * @s: substring_t to be scanned
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] parser: add unsigned int parser
  2021-01-28  2:20 [PATCH 3/3] parser: add unsigned int parser bingjingc
@ 2021-01-28  3:27 ` Randy Dunlap
  2021-01-28  8:32 ` Miklos Szeredi
  1 sibling, 0 replies; 4+ messages in thread
From: Randy Dunlap @ 2021-01-28  3:27 UTC (permalink / raw)
  To: bingjingc, viro, jack, jack, axboe, linux-fsdevel
  Cc: linux-kernel, cccheng, robbieko

Hi,

On 1/27/21 6:20 PM, bingjingc wrote:
> From: BingJing Chang <bingjingc@synology.com>
> 
> Will be used by fs parsing options
> 
> Reviewed-by: Robbie Ko<robbieko@synology.com>
> Reviewed-by: Chung-Chiang Cheng <cccheng@synology.com>
> Signed-off-by: BingJing Chang <bingjingc@synology.com>
> ---
>  fs/isofs/inode.c       | 16 ++--------------
>  fs/udf/super.c         | 16 ++--------------
>  include/linux/parser.h |  1 +
>  lib/parser.c           | 22 ++++++++++++++++++++++
>  4 files changed, 27 insertions(+), 28 deletions(-)

[snip]

> diff --git a/lib/parser.c b/lib/parser.c
> index f5b3e5d..2ec9c4f 100644
> --- a/lib/parser.c
> +++ b/lib/parser.c
> @@ -189,6 +189,28 @@ int match_int(substring_t *s, int *result)
>  EXPORT_SYMBOL(match_int);
>  
>  /**
> + * match_uint: - scan a decimal representation of an integer from a substring_t

This shows us that all of the kernel-doc for functions in
lib/parser.c is formatted incorrectly.

The above should be:

 * match_uint - scan a decimal representation of an integer from a substring_t

i.e., drop the ':' only on the function name lines, for all functions in
this source file.


If you don't want to do that, I'll plan to do it.


> + * @s: substring_t to be scanned
> + * @result: resulting integer on success
> + *
> + * Description: Attempts to parse the &substring_t @s as a decimal integer. On
> + * success, sets @result to the integer represented by the string and returns 0.
> + * Returns -ENOMEM, -EINVAL, or -ERANGE on failure.
> + */
> +int match_uint(substring_t *s, unsigned int *result)
> +{
> +	int err = -ENOMEM;
> +	char *buf = match_strdup(s);
> +
> +	if (buf) {
> +		err = kstrtouint(buf, 10, result);
> +		kfree(buf);
> +	}
> +	return err;
> +}
> +EXPORT_SYMBOL(match_uint);
> +
> +/**
>   * match_u64: - scan a decimal representation of a u64 from
>   *                  a substring_t

ditto.

>   * @s: substring_t to be scanned
> 


thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] parser: add unsigned int parser
  2021-01-28  2:20 [PATCH 3/3] parser: add unsigned int parser bingjingc
  2021-01-28  3:27 ` Randy Dunlap
@ 2021-01-28  8:32 ` Miklos Szeredi
       [not found]   ` <939d2196-8468-4d93-b976-70f3d8ac83de@Mail>
  1 sibling, 1 reply; 4+ messages in thread
From: Miklos Szeredi @ 2021-01-28  8:32 UTC (permalink / raw)
  To: bingjingc
  Cc: Al Viro, Jan Kara, Jan Kara, Jens Axboe, linux-fsdevel,
	linux-kernel, cccheng, robbieko

On Thu, Jan 28, 2021 at 3:21 AM bingjingc <bingjingc@synology.com> wrote:
>
> From: BingJing Chang <bingjingc@synology.com>
>
> Will be used by fs parsing options
>
> Reviewed-by: Robbie Ko<robbieko@synology.com>
> Reviewed-by: Chung-Chiang Cheng <cccheng@synology.com>
> Signed-off-by: BingJing Chang <bingjingc@synology.com>
> ---
>  fs/isofs/inode.c       | 16 ++--------------
>  fs/udf/super.c         | 16 ++--------------
>  include/linux/parser.h |  1 +
>  lib/parser.c           | 22 ++++++++++++++++++++++
>  4 files changed, 27 insertions(+), 28 deletions(-)
>
> diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> index 342ac19..21edc42 100644
> --- a/fs/isofs/inode.c
> +++ b/fs/isofs/inode.c
> @@ -335,18 +335,6 @@ static const match_table_t tokens = {
>         {Opt_err, NULL}
>  };
>
> -static int isofs_match_uint(substring_t *s, unsigned int *res)
> -{
> -       int err = -ENOMEM;
> -       char *buf = match_strdup(s);
> -
> -       if (buf) {
> -               err = kstrtouint(buf, 10, res);
> -               kfree(buf);
> -       }
> -       return err;
> -}

I don't see how adding this function and removing it in the same
series makes any sense.

Why not make this the first patch in the series, simplifying everything?

And while at it the referenced fuse implementation can also be
converted (as a separate patch).

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] parser: add unsigned int parser
       [not found]   ` <939d2196-8468-4d93-b976-70f3d8ac83de@Mail>
@ 2021-01-29  6:00     ` bingjing chang
  0 siblings, 0 replies; 4+ messages in thread
From: bingjing chang @ 2021-01-29  6:00 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Al Viro, Jan Kara, Jan Kara, Jens Axboe, linux-fsdevel,
	linux-kernel, cccheng, robbieko, bingjingc

Hi Miklos,

Thank you for your mail. Please see my message below.

bingjingc <bingjingc@synology.com> 於 2021年1月29日 週五 下午1:50寫道:
>
> [loop bxxxjxxg@gmail.com] in order to reply in plain-text
> Miklos Szeredi <miklos@szeredi.hu> 於 2021-01-28 16:37 寫道:
>
> On Thu, Jan 28, 2021 at 3:21 AM bingjingc <bingjingc@synology.com> wrote:
> >
> > From: BingJing Chang <bingjingc@synology.com>
> >
> > Will be used by fs parsing options
> >
> > Reviewed-by: Robbie Ko<robbieko@synology.com>
> > Reviewed-by: Chung-Chiang Cheng <cccheng@synology.com>
> > Signed-off-by: BingJing Chang <bingjingc@synology.com>
> > ---
> >  fs/isofs/inode.c       | 16 ++--------------
> >  fs/udf/super.c         | 16 ++--------------
> >  include/linux/parser.h |  1 +
> >  lib/parser.c           | 22 ++++++++++++++++++++++
> >  4 files changed, 27 insertions(+), 28 deletions(-)
> >
> > diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> > index 342ac19..21edc42 100644
> > --- a/fs/isofs/inode.c
> > +++ b/fs/isofs/inode.c
> > @@ -335,18 +335,6 @@ static const match_table_t tokens = {
> >         {Opt_err, NULL}
> >  };
> >
> > -static int isofs_match_uint(substring_t *s, unsigned int *res)
> > -{
> > -       int err = -ENOMEM;
> > -       char *buf = match_strdup(s);
> > -
> > -       if (buf) {
> > -               err = kstrtouint(buf, 10, res);
> > -               kfree(buf);
> > -       }
> > -       return err;
> > -}
>
> I don't see how adding this function and removing it in the same
> series makes any sense.

That's true. Simple and clear is better.
I used to think that the acceptance of patch can be 3/3 or 2/3.
And I was not sure are there needs for making match_uint
as shared lib. So I made the first patch.

I simplify them. Please see the third patch, thanks!

>
> Why not make this the first patch in the series, simplifying everything?
>
> And while at it the referenced fuse implementation can also be
> converted (as a separate patch).
>
> Thanks,
> Miklos

Thanks,
BingJing

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-01-29  6:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-28  2:20 [PATCH 3/3] parser: add unsigned int parser bingjingc
2021-01-28  3:27 ` Randy Dunlap
2021-01-28  8:32 ` Miklos Szeredi
     [not found]   ` <939d2196-8468-4d93-b976-70f3d8ac83de@Mail>
2021-01-29  6:00     ` bingjing chang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).