All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Mike Marshall <hubcap@omnibond.com>
Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: [patch] orangefs: cleanup orangefs_debugfs_new_client_string()
Date: Fri, 16 Dec 2016 10:45:24 +0000	[thread overview]
Message-ID: <20161216104524.GA2637@elgon.mountain> (raw)

Several small things in this function:
1) If copy to user fails we should return -EFAULT not -EIO
2) Don't print an error message, just fail.  It's annoying to let the
   users fill up dmesg and especially for something small like this.
3) Remove a stray tab.
4) Preserve the error code if orangefs_prepare_debugfs_help_string()
   fails.
5) "return 0;" is more explicit and clear than "return ret;".

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/fs/orangefs/orangefs-debugfs.c b/fs/orangefs/orangefs-debugfs.c
index 27e75cf28b3a..409fa6b0d339 100644
--- a/fs/orangefs/orangefs-debugfs.c
+++ b/fs/orangefs/orangefs-debugfs.c
@@ -966,15 +966,9 @@ int orangefs_debugfs_new_client_string(void __user *arg)
 {
 	int ret;
 
-	ret = copy_from_user(&client_debug_array_string,
-                                     (void __user *)arg,
-                                     ORANGEFS_MAX_DEBUG_STRING_LEN);
-
-	if (ret != 0) {
-		pr_info("%s: CLIENT_STRING: copy_from_user failed\n",
-			__func__);
-		return -EIO;
-	}
+	if (copy_from_user(&client_debug_array_string, arg,
+			   ORANGEFS_MAX_DEBUG_STRING_LEN))
+		return -EFAULT;
 
 	/*
 	 * The real client-core makes an effort to ensure
@@ -988,17 +982,18 @@ int orangefs_debugfs_new_client_string(void __user *arg)
 	 */
 	client_debug_array_string[ORANGEFS_MAX_DEBUG_STRING_LEN - 1]  		'\0';
-	
+
 	pr_info("%s: client debug array string has been received.\n",
 		__func__);
 
 	if (!help_string_initialized) {
 
 		/* Build a proper debug help string. */
-		if (orangefs_prepare_debugfs_help_string(0)) {
+		ret = orangefs_prepare_debugfs_help_string(0);
+		if (ret) {
 			gossip_err("%s: no debug help string \n",
 				   __func__);
-			return -EIO;
+			return ret;
 		}
 
 	}
@@ -1011,7 +1006,7 @@ int orangefs_debugfs_new_client_string(void __user *arg)
 
 	help_string_initialized++;
 
-	return ret;
+	return 0;
 }
 
 int orangefs_debugfs_new_debug(void __user *arg) 

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Mike Marshall <hubcap@omnibond.com>
Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: [patch] orangefs: cleanup orangefs_debugfs_new_client_string()
Date: Fri, 16 Dec 2016 13:45:24 +0300	[thread overview]
Message-ID: <20161216104524.GA2637@elgon.mountain> (raw)

Several small things in this function:
1) If copy to user fails we should return -EFAULT not -EIO
2) Don't print an error message, just fail.  It's annoying to let the
   users fill up dmesg and especially for something small like this.
3) Remove a stray tab.
4) Preserve the error code if orangefs_prepare_debugfs_help_string()
   fails.
5) "return 0;" is more explicit and clear than "return ret;".

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/fs/orangefs/orangefs-debugfs.c b/fs/orangefs/orangefs-debugfs.c
index 27e75cf28b3a..409fa6b0d339 100644
--- a/fs/orangefs/orangefs-debugfs.c
+++ b/fs/orangefs/orangefs-debugfs.c
@@ -966,15 +966,9 @@ int orangefs_debugfs_new_client_string(void __user *arg)
 {
 	int ret;
 
-	ret = copy_from_user(&client_debug_array_string,
-                                     (void __user *)arg,
-                                     ORANGEFS_MAX_DEBUG_STRING_LEN);
-
-	if (ret != 0) {
-		pr_info("%s: CLIENT_STRING: copy_from_user failed\n",
-			__func__);
-		return -EIO;
-	}
+	if (copy_from_user(&client_debug_array_string, arg,
+			   ORANGEFS_MAX_DEBUG_STRING_LEN))
+		return -EFAULT;
 
 	/*
 	 * The real client-core makes an effort to ensure
@@ -988,17 +982,18 @@ int orangefs_debugfs_new_client_string(void __user *arg)
 	 */
 	client_debug_array_string[ORANGEFS_MAX_DEBUG_STRING_LEN - 1] =
 		'\0';
-	
+
 	pr_info("%s: client debug array string has been received.\n",
 		__func__);
 
 	if (!help_string_initialized) {
 
 		/* Build a proper debug help string. */
-		if (orangefs_prepare_debugfs_help_string(0)) {
+		ret = orangefs_prepare_debugfs_help_string(0);
+		if (ret) {
 			gossip_err("%s: no debug help string \n",
 				   __func__);
-			return -EIO;
+			return ret;
 		}
 
 	}
@@ -1011,7 +1006,7 @@ int orangefs_debugfs_new_client_string(void __user *arg)
 
 	help_string_initialized++;
 
-	return ret;
+	return 0;
 }
 
 int orangefs_debugfs_new_debug(void __user *arg) 

             reply	other threads:[~2016-12-16 10:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-16 10:45 Dan Carpenter [this message]
2016-12-16 10:45 ` [patch] orangefs: cleanup orangefs_debugfs_new_client_string() Dan Carpenter
2016-12-16 20:35 ` Mike Marshall
2016-12-16 20:35   ` Mike Marshall
2016-12-16 21:15   ` Dan Carpenter
2016-12-16 21:15     ` Dan Carpenter

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=20161216104524.GA2637@elgon.mountain \
    --to=dan.carpenter@oracle.com \
    --cc=hubcap@omnibond.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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.