From: BingJing Chang <[email protected]>
Will be used by fs parsing options
Reviewed-by: Robbie Ko<[email protected]>
Reviewed-by: Chung-Chiang Cheng <[email protected]>
Signed-off-by: BingJing Chang <[email protected]>
---
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
Hi,
On 1/27/21 6:20 PM, bingjingc wrote:
> From: BingJing Chang <[email protected]>
>
> Will be used by fs parsing options
>
> Reviewed-by: Robbie Ko<[email protected]>
> Reviewed-by: Chung-Chiang Cheng <[email protected]>
> Signed-off-by: BingJing Chang <[email protected]>
> ---
> 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
On Thu, Jan 28, 2021 at 3:21 AM bingjingc <[email protected]> wrote:
>
> From: BingJing Chang <[email protected]>
>
> Will be used by fs parsing options
>
> Reviewed-by: Robbie Ko<[email protected]>
> Reviewed-by: Chung-Chiang Cheng <[email protected]>
> Signed-off-by: BingJing Chang <[email protected]>
> ---
> 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
Hi Miklos,
Thank you for your mail. Please see my message below.
bingjingc <[email protected]> 於 2021年1月29日 週五 下午1:50寫道：
>
> [loop [email protected]] in order to reply in plain-text
> Miklos Szeredi <[email protected]> 於 2021-01-28 16:37 寫道：
>
> On Thu, Jan 28, 2021 at 3:21 AM bingjingc <[email protected]> wrote:
> >
> > From: BingJing Chang <[email protected]>
> >
> > Will be used by fs parsing options
> >
> > Reviewed-by: Robbie Ko<[email protected]>
> > Reviewed-by: Chung-Chiang Cheng <[email protected]>
> > Signed-off-by: BingJing Chang <[email protected]>
> > ---
> > 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