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

[Peter Oberparleiter]

Andrew Morton wrote:
> On Wed, 03 Jun 2009 17:26:22 +0200
> Peter Oberparleiter <oberpar@xxxxxxxxxxxxxxxxxx> 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?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/