Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()

[Chen Gang <gang.chen@asianux.com>]

On 05/28/2013 08:17 PM, David Howells wrote:
> Chen Gang <gang.chen@asianux.com> wrote:
> 
>> Your suggestion will improve the speed, but may merge "transferring
>> 'protocol' data" and "processing 'protocol' data" together.
> 
> Look at it this way: You're having to step very carefully because you are
> fully expecting the strings not to be NUL-terminated.  Therefore you probably
> avoid using string functions if you can.
> 
> In fact, looking at the code, why are you copying the data through an
> intermediate buffer at all?  Why not just copy directly to userspace:
> 
> 	int len;
> 	char buf[2];
> 
> 	if (!*lenp || *ppos) {
> 		*lenp = 0;
> 		return 0;
> 	}
> 	if (!write) {
> -		len = sprintf(buf, appldata_timer_active ? "1\n" : "0\n");
> +		const char *ptr = appldata_timer_active ? "1\n" : "0\n";
> +		size_t len = 2;
> 		if (len > *lenp)
> 			len = *lenp;
> 		if (copy_to_user(buffer, buf, len))

may: "if (copy_to_user(buffer, ptr, len))" ?

:-)

> 			return -EFAULT;
> 		goto out;
> 	}
> 
> Put like that, it's fairly obvious what is going on.
> 

It seems well fixed (still independent of processing 'protocol' data).

:-)


And how about below:

-------------------------------diff begin------------------------------


diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c
index bae0f40..27f200d 100644
--- a/arch/s390/appldata/appldata_base.c
+++ b/arch/s390/appldata/appldata_base.c
@@ -212,10 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,
 		return 0;
 	}
 	if (!write) {
-		len = sprintf(buf, appldata_timer_active ? "1\n" : "0\n");
-		if (len > *lenp)
-			len = *lenp;
-		if (copy_to_user(buffer, buf, len))
+		if (copy_to_user(buffer,
+				 appldata_timer_active ? "1\n" : "0\n",
+				 min(2, *lenp))
 			return -EFAULT;
 		goto out;
 	}

-------------------------------diff end--------------------------------


Thanks
-- 
Chen Gang

Asianux Corporation