Re: [PATCH V2 3/9] edac, mce_amd_inj: Modify flags attrigute to use string arguments

[Borislav Petkov <bp@alien8.de>]

On Tue, Jun 02, 2015 at 03:35:56PM -0500, Aravind Gopalakrishnan wrote:
> Use char values such as "hw" or "sw" to indicate the type of error
> injection to be performed.
> 
> Current flags attribute derives the meanings of values that can be
> programmed into it from asm/mce.h. Moving to defined strings for the
> atribute allows this module to be self sufficient and removes the
> dependency. Also, we can introduce new flags as and when needed without
> having to worry about conflicting with the flags already defined
> in asm/mce.h
> 
> Also, modify do_inject() to use the newly defined injection_type enum
> to figure out the injection mechanism we need to use
> 
> Suggested-by: Borislav Petkov <bp@suse.de>
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> ---
>  drivers/edac/mce_amd_inj.c | 82 ++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 72 insertions(+), 10 deletions(-)

...

> -static int flags_set(void *data, u64 val)
> +static ssize_t flags_write(struct file *filp, const char __user *ubuf,
> +			   size_t cnt, loff_t *ppos)
>  {
> -	struct mce *m = (struct mce *)data;
> +	char buf[MAX_FLAG_OPT_SIZE];
> +	int err;
> +	size_t ret;
>  
> -	m->inject_flags = (u8)val;
> -	return 0;
> +	if (cnt > MAX_FLAG_OPT_SIZE)
> +		cnt = MAX_FLAG_OPT_SIZE;
> +
> +	ret = cnt;
> +
> +	if (copy_from_user(&buf, ubuf, cnt))
> +		return -EFAULT;
> +
> +	buf[cnt - 1] = 0;
> +
> +	/* strip whitespaces.. */
> +	strstrip(buf);

Didn't your compiler trigger that:

drivers/edac/mce_amd_inj.c: In function ‘flags_write’:
drivers/edac/mce_amd_inj.c:146:2: warning: ignoring return value of ‘strstrip’, declared with attribute warn_unused_result [-Wunused-result]
  strstrip(buf);
  ^

?

Because it is a valid warning. You need to take the return value. I
fixed it up like this:

---
diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
index c129a8da34b2..5c847fe6e9bd 100644
--- a/drivers/edac/mce_amd_inj.c
+++ b/drivers/edac/mce_amd_inj.c
@@ -128,7 +128,7 @@ static ssize_t flags_read(struct file *filp, char __user *ubuf,
 static ssize_t flags_write(struct file *filp, const char __user *ubuf,
 			   size_t cnt, loff_t *ppos)
 {
-	char buf[MAX_FLAG_OPT_SIZE];
+	char buf[MAX_FLAG_OPT_SIZE], *__buf;
 	int err;
 	size_t ret;
 
@@ -142,12 +142,12 @@ static ssize_t flags_write(struct file *filp, const char __user *ubuf,
 
 	buf[cnt - 1] = 0;
 
-	/* strip whitespaces.. */
-	strstrip(buf);
+	/* strip whitespace */
+	__buf = strstrip(buf);
 
-	err = __set_inj(buf);
+	err = __set_inj(__buf);
 	if (err) {
-		pr_err("%s: Invalid flags value: %s\n", __func__, buf);
+		pr_err("%s: Invalid flags value: %s\n", __func__, __buf);
 		return err;
 	}


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--