6 Mar
2011
6 Mar
'11
10:40 p.m.
On Mar 7, 2011, at 12:43 AM, Pierre Schweitzer wrote:
Hi,
this commits just shows you didn't understand how that function
works. If you had, you wouldn't have said:
"- Implement parameters checking in FsRtlIsNameInExpressionPrivate."
It's already done.
Enlighten me please, where? Is it crypted in the last return
statement? If so, how many of you would decipher, it should return
TRUE if both Name and Expression are empty, and false if any of them
is not empty? And how many of you would break this in an attempt to
improve the algorithm and thus break that nice 119 chars return
statement?
"- Add two shortcuts for common wildcard invocations to make the
function faster."
If you look at the algorithm, it will take at maximum as much loop
iterations as name length.
Yeah, but not making those loop iterations is even
better for,
probably, 90% of function invocations. Also I will be honest: I'm not
sure that loop is correct but I'm not sure it's incorrect either ("a
joke about Schroedinger cat could be funny and not funny at the same
moment of time").
Constants updates to the code making it less and less readable only
add up to my concerns, with latest pool overruns too.
And in contrary, at the very least I am sure those parts I committed
are perfectly working code.
"- Second (main part of the function) is still under review."
Thanks for discussing the issue with the original author of the
algorithm, especially since you don't understand it. Furthermore,
if you're not happy with said algorithm, you were asked to review
it more than a year ago, and for at least 6 months. Thing that you
NEVER did.
I review my own code too, what's the matter? Also, if it looked fine
for me a year ago, it doesn't mean it's perfect and it should never
be reviewed again. For your information, I review even the most
perfectly working code in the kernel, and sometimes that's rewarded
by finding nice typos leading to unpredicted results in a not very
often executed branch of a code.
So, now you discovered what my mail address is, contact me before
committing anything wrong/useless to current code.
And as you said less than a week ago, focus on fixing things that
don't work, instead of rewriting working code.
Somehow I thought that 7+ years
of hacking this kernel (starting from
revision 4578, and 522 kernel commits over time) give me some idea of
what is correct, what is useful and what does not work.
And as someone else also said once: 1:1 isn't the solution
everywhere. Or if you think so, start dropping arwinss.
Yeah, I don't disagree
with my own words, however it was the case
when it's a good addition to the code. Maxim Shatskih even outlined
there is a bug in the original Windows implementation of that
function and suggested (no, not to us, suggested to someone in some
forum thread) to use something better "like grep source code".
Of course, don't take that personal, I like
everyone ;-).
If I would take anything personal, I wouldn't survive in this
project
for that long :D
WBR,
Aleksey.