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
[email protected] 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
