Messages in this thread | | | Date | Thu, 28 Dec 2006 19:41:25 +0100 | From | "Daniel Marjamäki" <> | 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@vger.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |