All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Whitehouse <swhiteho@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [RFC 1/1 linux-next] gfs2: convert simple_str to kstr
Date: Thu, 30 Apr 2015 10:28:03 +0100	[thread overview]
Message-ID: <5541F5A3.7080108@redhat.com> (raw)
In-Reply-To: <1430333674-2266-1-git-send-email-fabf@skynet.be>

Hi,

On 29/04/15 19:54, Fabian Frederick wrote:
> -Remove obsolete simple_str functions.
> -Return error code when kstr failed.
> -This patch also calls functions corresponding to destination type.
>
> Basically it's an RFC because of the type mismatch all over the place.
> ie code was doing simple_strtol to integer...
> Newer kstr functions detect such problems.
> By default I used destination type as a reference. Maybe it's wrong and we
> really want to read long, unsigned long from source ?
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Looks correct to me. Most of those are simple 1 or 0 interfaces anyway.
Acked-by: Steven Whitehouse <swhiteho@redhat.com>

Steve.

> ---
>   fs/gfs2/sys.c | 56 ++++++++++++++++++++++++++++++++++++++++++++------------
>   1 file changed, 44 insertions(+), 12 deletions(-)
>
> diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
> index ae8e881..436bdeb 100644
> --- a/fs/gfs2/sys.c
> +++ b/fs/gfs2/sys.c
> @@ -101,8 +101,11 @@ static ssize_t freeze_show(struct gfs2_sbd *sdp, char *buf)
>   
>   static ssize_t freeze_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   {
> -	int error;
> -	int n = simple_strtol(buf, NULL, 0);
> +	int error, n;
> +
> +	error = kstrtoint(buf, 0, &n);
> +	if (error)
> +		return error;
>   
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
> @@ -134,10 +137,16 @@ static ssize_t withdraw_show(struct gfs2_sbd *sdp, char *buf)
>   
>   static ssize_t withdraw_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   {
> +	int error, val;
> +
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	if (simple_strtol(buf, NULL, 0) != 1)
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
> +
> +	if (val != -1)
>   		return -EINVAL;
>   
>   	gfs2_lm_withdraw(sdp, "withdrawing from cluster at user's request\n");
> @@ -148,10 +157,16 @@ static ssize_t withdraw_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   static ssize_t statfs_sync_store(struct gfs2_sbd *sdp, const char *buf,
>   				 size_t len)
>   {
> +	int error, val;
> +
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	if (simple_strtol(buf, NULL, 0) != 1)
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
> +
> +	if (val != -1)
>   		return -EINVAL;
>   
>   	gfs2_statfs_sync(sdp->sd_vfs, 0);
> @@ -161,10 +176,16 @@ static ssize_t statfs_sync_store(struct gfs2_sbd *sdp, const char *buf,
>   static ssize_t quota_sync_store(struct gfs2_sbd *sdp, const char *buf,
>   				size_t len)
>   {
> +	int error, val;
> +
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	if (simple_strtol(buf, NULL, 0) != 1)
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
> +
> +	if (val != -1)
>   		return -EINVAL;
>   
>   	gfs2_quota_sync(sdp->sd_vfs, 0);
> @@ -181,7 +202,9 @@ static ssize_t quota_refresh_user_store(struct gfs2_sbd *sdp, const char *buf,
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	id = simple_strtoul(buf, NULL, 0);
> +	error = kstrtou32(buf, 0, &id);
> +	if (error)
> +		return error;
>   
>   	qid = make_kqid(current_user_ns(), USRQUOTA, id);
>   	if (!qid_valid(qid))
> @@ -201,7 +224,9 @@ static ssize_t quota_refresh_group_store(struct gfs2_sbd *sdp, const char *buf,
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	id = simple_strtoul(buf, NULL, 0);
> +	error = kstrtou32(buf, 0, &id);
> +	if (error)
> +		return error;
>   
>   	qid = make_kqid(current_user_ns(), GRPQUOTA, id);
>   	if (!qid_valid(qid))
> @@ -325,9 +350,11 @@ static ssize_t block_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   {
>   	struct lm_lockstruct *ls = &sdp->sd_lockstruct;
>   	ssize_t ret = len;
> -	int val;
> +	int val, error;
>   
> -	val = simple_strtol(buf, NULL, 0);
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
>   
>   	if (val == 1)
>   		set_bit(DFL_BLOCK_LOCKS, &ls->ls_recover_flags);
> @@ -351,9 +378,11 @@ static ssize_t wdack_show(struct gfs2_sbd *sdp, char *buf)
>   static ssize_t wdack_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   {
>   	ssize_t ret = len;
> -	int val;
> +	int val, error;
>   
> -	val = simple_strtol(buf, NULL, 0);
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
>   
>   	if ((val == 1) &&
>   	    !strcmp(sdp->sd_lockstruct.ls_ops->lm_proto_name, "lock_dlm"))
> @@ -553,11 +582,14 @@ static ssize_t tune_set(struct gfs2_sbd *sdp, unsigned int *field,
>   {
>   	struct gfs2_tune *gt = &sdp->sd_tune;
>   	unsigned int x;
> +	int error;
>   
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	x = simple_strtoul(buf, NULL, 0);
> +	error = kstrtouint(buf, 0, &x);
> +	if (error)
> +		return error;
>   
>   	if (check_zero && !x)
>   		return -EINVAL;



WARNING: multiple messages have this Message-ID (diff)
From: Steven Whitehouse <swhiteho@redhat.com>
To: Fabian Frederick <fabf@skynet.be>, linux-kernel@vger.kernel.org
Cc: Bob Peterson <rpeterso@redhat.com>, cluster-devel@redhat.com
Subject: Re: [RFC 1/1 linux-next] gfs2: convert simple_str to kstr
Date: Thu, 30 Apr 2015 10:28:03 +0100	[thread overview]
Message-ID: <5541F5A3.7080108@redhat.com> (raw)
In-Reply-To: <1430333674-2266-1-git-send-email-fabf@skynet.be>

Hi,

On 29/04/15 19:54, Fabian Frederick wrote:
> -Remove obsolete simple_str functions.
> -Return error code when kstr failed.
> -This patch also calls functions corresponding to destination type.
>
> Basically it's an RFC because of the type mismatch all over the place.
> ie code was doing simple_strtol to integer...
> Newer kstr functions detect such problems.
> By default I used destination type as a reference. Maybe it's wrong and we
> really want to read long, unsigned long from source ?
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Looks correct to me. Most of those are simple 1 or 0 interfaces anyway.
Acked-by: Steven Whitehouse <swhiteho@redhat.com>

Steve.

> ---
>   fs/gfs2/sys.c | 56 ++++++++++++++++++++++++++++++++++++++++++++------------
>   1 file changed, 44 insertions(+), 12 deletions(-)
>
> diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
> index ae8e881..436bdeb 100644
> --- a/fs/gfs2/sys.c
> +++ b/fs/gfs2/sys.c
> @@ -101,8 +101,11 @@ static ssize_t freeze_show(struct gfs2_sbd *sdp, char *buf)
>   
>   static ssize_t freeze_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   {
> -	int error;
> -	int n = simple_strtol(buf, NULL, 0);
> +	int error, n;
> +
> +	error = kstrtoint(buf, 0, &n);
> +	if (error)
> +		return error;
>   
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
> @@ -134,10 +137,16 @@ static ssize_t withdraw_show(struct gfs2_sbd *sdp, char *buf)
>   
>   static ssize_t withdraw_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   {
> +	int error, val;
> +
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	if (simple_strtol(buf, NULL, 0) != 1)
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
> +
> +	if (val != -1)
>   		return -EINVAL;
>   
>   	gfs2_lm_withdraw(sdp, "withdrawing from cluster at user's request\n");
> @@ -148,10 +157,16 @@ static ssize_t withdraw_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   static ssize_t statfs_sync_store(struct gfs2_sbd *sdp, const char *buf,
>   				 size_t len)
>   {
> +	int error, val;
> +
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	if (simple_strtol(buf, NULL, 0) != 1)
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
> +
> +	if (val != -1)
>   		return -EINVAL;
>   
>   	gfs2_statfs_sync(sdp->sd_vfs, 0);
> @@ -161,10 +176,16 @@ static ssize_t statfs_sync_store(struct gfs2_sbd *sdp, const char *buf,
>   static ssize_t quota_sync_store(struct gfs2_sbd *sdp, const char *buf,
>   				size_t len)
>   {
> +	int error, val;
> +
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	if (simple_strtol(buf, NULL, 0) != 1)
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
> +
> +	if (val != -1)
>   		return -EINVAL;
>   
>   	gfs2_quota_sync(sdp->sd_vfs, 0);
> @@ -181,7 +202,9 @@ static ssize_t quota_refresh_user_store(struct gfs2_sbd *sdp, const char *buf,
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	id = simple_strtoul(buf, NULL, 0);
> +	error = kstrtou32(buf, 0, &id);
> +	if (error)
> +		return error;
>   
>   	qid = make_kqid(current_user_ns(), USRQUOTA, id);
>   	if (!qid_valid(qid))
> @@ -201,7 +224,9 @@ static ssize_t quota_refresh_group_store(struct gfs2_sbd *sdp, const char *buf,
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	id = simple_strtoul(buf, NULL, 0);
> +	error = kstrtou32(buf, 0, &id);
> +	if (error)
> +		return error;
>   
>   	qid = make_kqid(current_user_ns(), GRPQUOTA, id);
>   	if (!qid_valid(qid))
> @@ -325,9 +350,11 @@ static ssize_t block_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   {
>   	struct lm_lockstruct *ls = &sdp->sd_lockstruct;
>   	ssize_t ret = len;
> -	int val;
> +	int val, error;
>   
> -	val = simple_strtol(buf, NULL, 0);
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
>   
>   	if (val == 1)
>   		set_bit(DFL_BLOCK_LOCKS, &ls->ls_recover_flags);
> @@ -351,9 +378,11 @@ static ssize_t wdack_show(struct gfs2_sbd *sdp, char *buf)
>   static ssize_t wdack_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
>   {
>   	ssize_t ret = len;
> -	int val;
> +	int val, error;
>   
> -	val = simple_strtol(buf, NULL, 0);
> +	error = kstrtoint(buf, 0, &val);
> +	if (error)
> +		return error;
>   
>   	if ((val == 1) &&
>   	    !strcmp(sdp->sd_lockstruct.ls_ops->lm_proto_name, "lock_dlm"))
> @@ -553,11 +582,14 @@ static ssize_t tune_set(struct gfs2_sbd *sdp, unsigned int *field,
>   {
>   	struct gfs2_tune *gt = &sdp->sd_tune;
>   	unsigned int x;
> +	int error;
>   
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EPERM;
>   
> -	x = simple_strtoul(buf, NULL, 0);
> +	error = kstrtouint(buf, 0, &x);
> +	if (error)
> +		return error;
>   
>   	if (check_zero && !x)
>   		return -EINVAL;


  reply	other threads:[~2015-04-30  9:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-29 18:54 [Cluster-devel] [RFC 1/1 linux-next] gfs2: convert simple_str to kstr Fabian Frederick
2015-04-29 18:54 ` Fabian Frederick
2015-04-30  9:28 ` Steven Whitehouse [this message]
2015-04-30  9:28   ` Steven Whitehouse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5541F5A3.7080108@redhat.com \
    --to=swhiteho@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.