Date:	Thu, 28 Dec 2006 19:41:25 +0100
From:	"Daniel Marjamäki" <daniel.marjamaki@...il.com>
To:	linux-kernel@...r.kernel.org
Subject: Want comments regarding patch

Hello all!

I sent a patch with this content:

-       for (i = 0; i < MAX_PIRQS; i++)
-               pirq_entries[i] = -1;
+       memset(pirq_entries, -1, sizeof(pirq_entries));

I'd like to know if you have any comments to this change. It was of
course my intention to make the code shorter, simpler and faster.

I've discussed this with Ingo Molnar and here's our conversation:

INGO:

hm, i'm not sure i like this - the '-1' in the memset is for a byte,
while the pirq_entries are word sized. It should work because the bytes
happen to be 0xff for the word too, but this is encodes an assumption,
and were we ever to change that value it could break silently. gcc ought
to be able to figure out the best way to initialize the array.


DANIEL:

Thank you for the comments.

I understand your point, it's good. But I personally still like my
method better.
For me -1 is just as valid as an argument as 0. As you note however,
it assumes that the next developer understands the encoding of
negative numbers. A developer who doesn't know the encoding could be
very confused. Would my patch be ok if I used '0xff' instead of '-1'?

With version 3.3.6 (gcc) there's quite a big difference in the
assembly code (between 'for' and 'memset').

INGO:

0xff might be better, but i'm still uneasy about it ... No other piece
of code within the kernel does this.

could you post the patch and the reasoning to
linux-kernel@...r.kernel.org as well? That way others can chime in as
well.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists