Re: [PATCH 3/4] gcov: add gcov profiling infrastructure

[Peter Oberparleiter <oberpar@linux.vnet.ibm.com>]

Andrew Morton wrote:
> On Wed, 03 Jun 2009 17:26:22 +0200
> Peter Oberparleiter <oberpar@linux.vnet.ibm.com> wrote:
> 
>>> Right - the sscanf would make sense if kernel parameters could contain 
>>> spaces (in that case it catches <number><blanks><garbage> input) which 
>>> it can't so strtoul() would indeed make more sense. I'll prepare an 
>>> updated patch and send it out later today.
>> See below for the updated patch that uses strtoul instead of sscanf.
>> This patch replaces kernel-constructor-support.patch in the -mm tree:
> 
> umm, no it doesn't.  I get the below incremental patch, against
> gcov-add-gcov-profiling-infrastructure.patch:

Ah, right. That answers my question whether further changes should be 
posted as complete or incremental patches.

> --- a/kernel/gcov/fs.c~gcov-add-gcov-profiling-infrastructure-update
> +++ a/kernel/gcov/fs.c
> @@ -70,15 +70,8 @@ static int gcov_persist = 1;
> 
>  static int __init gcov_persist_setup(char *str)
>  {
> -	int val;
> -	char delim;
> -
> -	if (sscanf(str, "%d %c", &val, &delim) != 1) {
> -		pr_warning("invalid gcov_persist parameter '%s'\n", str);
> -		return 0;
> -	}
> -	pr_info("setting gcov_persist to %d\n", val);
> -	gcov_persist = val;
> +	gcov_persist = simple_strtoul(str, NULL, 0);
> +	pr_info("setting gcov_persist to %d\n", gcov_persist);
> 
>  	return 1;
>  }
> _
> 
> arguably we should use strict_strtoul(), but the kernel is a lot less
> fussy about boot parameters than it is with sysfs writes, etc.  If you
> fat-finger your grub.conf, you lose and we don't tell you.

Agreed. Which leads me to another question: is it ok to post many small 
patches for -mm for minor changes like this one (simple_ -> strict_) or 
do you rather prefer these to be collected into somewhat larger patches 
that fix multiple minor issues?