[ros-dev] [ros-diffs] [fireball] 50981: [NTOS/FSRTL] - Implement parameters checking i

[Aleksey Bragin]

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.