Re: [PATCH] mtd: fix memory leaks in phram_setup

[Ingo Oeser <ioe-lkml@rameria.de>]

Hi Jörn,

On Sunday, 14. May 2006 15:21, Jörn Engel wrote:
> On Sun, 14 May 2006 12:03:49 +0100, David Woodhouse wrote:
> > On Sun, 2006-05-14 at 08:37 +0200, Jörn Engel wrote:
> > > The only question is: does it make the code better?  The code has
> > > seven printk/return combinations.  Each of them would chew up 2 more
> > > lines without the macro.  So phram_setup would grow from 44 to 58
> > > lines, not nice either.
> > 
> > How about this then...
> 
> Moving the printk into leaf functions?  My plan still is to collect a
> bunch of those and put somewhere in lib/.  So maybe that's a bad idea.

That has been done already. One is kstrdup() from <linux/string.h>
implementation in mm/util.c . So duplication can go.

parse_num32 is implemented as memparse() from 
<linux/kernel.h> already, code is in lib/cmdline.c

It only misses the SI prefix stuff (kiBi and such).

Maybe SI units could be added to memparse() in
lib/cmdline.c and all those reimplementations ripped of drivers/mtd/*/* ?

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 0331ed8..d13c580 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -108,6 +108,8 @@ unsigned long long memparse (char *ptr, 
 	case 'k':
 		ret <<= 10;
 		(*retptr)++;
+		if ((**retptr) == 'i')
+			(*retptr)++;
 	default:
 		break;
 	}

> The macro sucks. 
Agreed stronly! It is also against Documentation/CodingStyle, Chapter 11

No worries, error handling always clutters up our nice code. 

The only accepted way around this in Linux is "goto error_label", 
so we can see the algorithm and read up on error handling at the 
end of a function, after the algorithm.


Regards

Ingo Oeser