'I read this code and it looked clean to me' line means that the commiter read the code, and assures all or some parts of the following: 1. The code doesn't match any reverse-engineered rules (as on wiki page regarding Audit) 2. The code is publically documented 3. The code has nothing to do with reverse engineering (has either completely different implementation from the windows one - example freeldr vs. ntldr/osloader, or doesn't have any counterpart in windows at all).
It gives a good base point for us to start our defence from.
We are not under attack. We are just doing some preventive measures.
This was all decided when we originally locked the code, but no one has been following it.
arty, w3seek, me have been following this rules on the possibly dirty code, so please don't speak for everyone. Howeve, I don't feel like documenting every function of e.g. dll/cpl/intl because I am sure that reversing this component is a waste of time, and no reverser would ever do this. In such case, simple code review IS possible, to ensure the code doesn't contain reverse engineered items. If you like to document those parts - feel free to do so, I am not against this.
In light of this, I think some of the audit measures we went to were a bit of an over reaction. Perhaps we should have only decided to audit kernel mode code, or subsections of it.
That's true, however going through all code produces better results. Certainly percentage of found dirty code will be very-very low, and most of the auditing commits will be unlocking clean stuff. That's how we originally thought the process to be.
If someone have something against the unlocked parts - tell which parts are wrong, I will personally review them, or lock again if one insists.
With the best regards, Aleksey Bragin.
----- Original Message ----- From: "Murphy, Ged (Bolton)" MurphyG@cmpbatteries.co.uk To: "'ReactOS Development List'" ros-dev@reactos.org Sent: Friday, April 07, 2006 3:20 PM Subject: RE: [ros-dev] ncpa
If the author has been contacted, it should be noted in the commit log. That alone is enough reason to squash any doubts anyone had. The authors word is always trusted.
If the author couldn't be contacted, then the code should go through the audit procedure Art put forward. Whichever part of that procedure was passed should be noted in the commit log. Even if it's just something as simple as 'this code uses is fully documented on XYZ: http://..'
Doing things this way ensures we can look back at the code if the cleanliness of the reversing methods is ever questioned again. It gives a good base point for us to start our defence from.
This was all decided when we originally locked the code, but no one has been following it. Thus we have terms like 'I read this code and it looked clean to me'. IMO, that isn't worth anything. I could read parts of the kernel and it appear to be clean to me. Contacting Alex or correctly auditing the code would prove otherwise.
In light of this, I think some of the audit measures we went to were a bit of an over reaction. Perhaps we should have only decided to audit kernel mode code, or subsections of it. However we choose to do everything and I'm a firm believer in the phrase 'If you going to do something, do it right'
Ged.
-----Original Message----- From: Saveliy Tretiakov [mailto:saveliyt@mail.ru] Sent: 07 April 2006 13:52 To: ReactOS Development List Subject: Re: [ros-dev] ncpa
I always contact authors when it is possible. Some authors have left project years ago and are unreachable.
Murphy, Ged (Bolton) wrote:
The authors of the code should always be contacted before unlocking unless the app it produces isn't a part of Windows.
Not doing so undermines the audit process and commit lines like 'I looked
at
this code and it appears clean' is doing just that.
Ged.
-----Original Message----- From: Saveliy Tretiakov [mailto:saveliyt@mail.ru] Sent: 07 April 2006 13:11 To: ReactOS Development List Subject: Re: [ros-dev] ncpa
Do we need to audit ncpa?