Re: [PATCH][trivial] crypto: tcrypt - reduce stack size

[Neil Horman]

On Wed, Feb 25, 2009 at 02:48:19PM +0100, Frank Seidel wrote:
> From: Frank Seidel <frank@xxxxxxxxxxx>
> 
> Applying kernel janitors todos (printk calls need KERN_*
> constants on linebeginnings, reduce stack footprint where
> possible) to tcrypts test_hash_speed (where stacks
> memory footprint was very high (on i386 1184 bytes to
> 292 now).
> 
> Signed-off-by: Frank Seidel <frank@xxxxxxxxxxx>
> ---
>  
>  	if (IS_ERR(tfm)) {
> -		printk("failed to load transform for %s: %ld\n", algo,
> +		printk(KERN_ERR "failed to load transform for %s: %ld\n", algo,
>  		       PTR_ERR(tfm));
> +		kfree(output);
>  		return;
Its just a stylistic nit, but I think I'd rather see this be a goto to the kfree
at the end of the function.  You can move the out label down to just the kfree,
and rename the current out usage to out_free_tfm.  I think if you're going to
add the kfree at the bottom, you might as well make use of it for all cases.

If you change that, I'll ack this.

Thanks & Regards
Neil

>  	}
>  
>  	desc.tfm = tfm;
>  	desc.flags = 0;
>  
> -	if (crypto_hash_digestsize(tfm) > sizeof(output)) {
> -		printk("digestsize(%u) > outputbuffer(%zu)\n",
> -		       crypto_hash_digestsize(tfm), sizeof(output));
> +	if (crypto_hash_digestsize(tfm) > output_size) {
> +		printk(KERN_ERR "digestsize(%u) > outputbuffer(%zu)\n",
> +		       crypto_hash_digestsize(tfm), output_size);
>  		goto out;
>  	}
>  
> @@ -427,12 +435,14 @@ static void test_hash_speed(const char *
>  
>  	for (i = 0; speed[i].blen != 0; i++) {
>  		if (speed[i].blen > TVMEMSIZE * PAGE_SIZE) {
> -			printk("template (%u) too big for tvmem (%lu)\n",
> +			printk(KERN_ERR
> +			       "template (%u) too big for tvmem (%lu)\n",
>  			       speed[i].blen, TVMEMSIZE * PAGE_SIZE);
>  			goto out;
>  		}
>  
> -		printk("test%3u (%5u byte blocks,%5u bytes per update,%4u updates): ",
> +		printk(KERN_INFO "test%3u "
> +		       "(%5u byte blocks,%5u bytes per update,%4u updates): ",
>  		       i, speed[i].blen, speed[i].plen, speed[i].blen / speed[i].plen);
>  
>  		if (sec)
> @@ -443,13 +453,14 @@ static void test_hash_speed(const char *
>  					       speed[i].plen, output);
>  
>  		if (ret) {
> -			printk("hashing failed ret=%d\n", ret);
> +			printk(KERN_ERR "hashing failed ret=%d\n", ret);
>  			break;
>  		}
>  	}
>  
>  out:
>  	crypto_free_hash(tfm);
> +	kfree(output);
>  }
>  
>  static void test_available(void)
> 
--
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/