[Cbe-oss-dev] [PATCH] OProfile fix call to kzalloc

[Stephen Rothwell]

On Fri, 22 Jun 2007 11:21:31 -0500 Bob Nelson <rrnelson at linux.vnet.ibm.com> wrote:
>  
> -static u32 * samples;
> +static u32 * samples = 0;

We don't initialize globals to 0 (that is done for us) and in any case it
should have been NULL (since it is a pointer).  However we also don't
separate the '*' form the variable name (if you really want to fix this
line).

>  static int spu_prof_running = 0;
>  static unsigned int profiling_interval = 0;

So, I wonder why these are initialized.  :-)

> -    spu_prof_running = 1;
> -    hrtimer_start(&timer, kt, HRTIMER_MODE_REL);
> +    if (unlikely(samples == 0)) {
> +        ret = -ENOMEM;
> +    } else {
> +        spu_prof_running = 1;
> +        hrtimer_start(&timer, kt, HRTIMER_MODE_REL);
> +    }
> +
> +        return ret;

You might as well do
	if (unlikely(samples == NULL))
		return -ENOMEM;
	.
	.
	return 0;

so you don't need the ret variable as the code is really quite simple.


>  void stop_spu_profiling(void)
>  {
>      spu_prof_running = 0;
>      hrtimer_cancel(&timer);
> -    kfree(samples);
> +    if (samples != 0) {
> +        kfree(samples);
> +    }

kfree(NULL) is fine (and we have been removing the tests for NULL before
kfree elsewhere). Also we don't bracket single statements normally.

-- 
Cheers,
Stephen Rothwell                    sfr at canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/cbe-oss-dev/attachments/20070623/9515ae27/attachment.pgp>