Re: [PATCH] include/linux/slab.h: new KFREE() macro.

On Sun, 2007-01-07 at 16:02 -0800, Amit Choudhary wrote:
That's where KFREE(ptr) comes in so that the code doesn't look ugly and still the purpose is
achieved.

Shoving it into a macro makes it no better.

Reading code like that makes me say "wtf?", simply because 'ptr' is not
used thereafter,

Really? Then why do we have all the debugging options to catch re-use of the memory that has been
freed. So many debugging options has been implemented, so much effort has gone into them, partly
because programmers sometimes miss correct programming.

Which is exactly why we should continue to let programmers focus on
trying to get their code correct and letting the existing safety tools
catch thinkos, instead of distracting them with confusing and completely
pointless variable assignments.

I do not know what you are talking about here. You are saying that a function does not need three
different arrays with different names. How can you say that? How do you know what is the
requirement?

What I said was that your example proves something entirely different
than what you think: rather than proving the need for your KFREE()
macro, it instead proves the need to design the code correctly from the
start, so as to avoid even thinking about this crud.

If you insist, here's your example function, trivially rewritten without
any NULL assignments. (I used two arrays, not three, to save space. The
basic idea works by-design for any random number of arrays, each of any
arbitrary size.)

struct type1 {
/* something */
};

struct type2 {
/* something */
};

#define COUNT 10

void function1(struct type1 **a1, struct type2 **a2, unsigned int sz);

void function2(void)
{
struct type1 *arr1[COUNT];
struct type2 *arr2[COUNT];
int i;

/* init */
for (i = 0; i < COUNT; i++) {
arr1[i] = kmalloc(sizeof(struct type1), ...);
if (!arr1[i])
goto free_1;
}
for (i = 0; i < COUNT; i++) {
arr2[i] = kmalloc(sizeof(struct type2), ...);
if (!arr2[i])
goto free_2;
}

/* work */
function1(arr1, arr2, COUNT);

/* fini */
i = COUNT;
free_2:
for (i--; i >= 0; i--) {
kfree(arr2[i]);
}
i = COUNT;
free_1:
for (i--; i >= 0; i--) {
kfree(arr1[i]);
}
}

In most cases, though, the above code design would be brain-damaged from
the start: unless the sizes involved are prohibitively large, the
function should be allocating all the memory in a single pass.

So, where's the demonstrated need for KFREE()?

-- Vadim Lobanov


-
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/

Relevant Pages

  • Re: ARRAY QUESTION...
    ... I'm working on a database right now and am writing a macro that will perform ... I'm not good at all with arrays and have no idea how ... Sub EmptyAllOutputTables() ... Dim tdf As DAO.TableDef ...
    (microsoft.public.access.modulesdaovba)
  • Re: How do I assign values to an array?
    ... >The advantage is that MyValue is type Single() as specified by the ... I anticipated the OP's response that your Assign macro was too complicated. ... There *IS* both performance drag and additional storage overhead storing dynamic ... large arrays, in which case the function call and processing overhead would ...
    (microsoft.public.excel.programming)
  • Re: [SICP] newbie question about implementing pairs
    ... macro processors in assemblers in those days were Turing complete ... "structures" to turn it into parallel arrays of structure components, ... do (setf (aref names i) name) ...
    (comp.lang.lisp)
  • Re: C Coding style question
    ... const int arrSize = ARRAY_SIZE; ... and use arrSize throughout the code. ... Only way you can do that is state that the project must have ALL arrays ... If I use the macro directly I have ...
    (comp.arch.embedded)
  • Fill color in autoshape based on cell data
    ... I was thinking a select case macro, but can't seem to get the code correct. ... I am a novice at macros. ... Thanks for any assistance. ...
    (microsoft.public.excel.misc)