On 07/25/2010 01:34 PM, Andrew Eikum wrote:
On 07/25/2010 12:04 PM, Max TenEyck Woodbury wrote:
On 07/25/2010 09:45 AM, James McKenzie wrote:
I think you missed what Nicolay and Dmitry are trying to tell you. We are trying to implement, bug for bug, the functionality of what Windows does. Does Windows return "STATUS_NOT_IMPLEMENTED" when this call is made? If not, your fix is WRONG. Silencing a 'fixme' is NOT a fix and this will be REJECTED. If this is correct and is what Windows does, then state so. Otherwise, withdraw the patch and fix it the right way.
James McKenzie
Frankly, I do not know what Microsoft does, but the test would fail on their implementation if they did something else, so I think it is safe to assume the test is implemented properly. Given that, the fixme is wrong.
Specifically, Nicolay asked for a test case. Since I was working from an already existing test case, his request didn't really make sense. I pointed out that there already was a test case and that should have been the end of it.
Well, you didn't point out which existing testcase you are talking about. All that your patch does is silence a FIXME. Presumably, the FIXME was placed there for a reason. Nikolay and Dmitry were pointing out that silencing that FIXME might not be appropriate, and were asking for you to demonstrate why that FIXME is invalid. Adding a testcase or pointing to an existing testcase would accomplish this.
Which existing testcase demonstrates that this behavior is valid and that the FIXME is unwarranted? Does the existing testcase demonstrate the full range of behavior given that parameter? Can you expand on the tests to show that your implementation is always correct?
From my notes (line breaks added to prevent wrapping)
fixme:ntdll:NtSetInformationFile Unsupported class (18) dlls/ntdll/file.c:2152 in NtSetInformationFile NTSTATUS WINAPI NtSetInformationFile(HANDLE handle, PIO_STATUS_BLOCK io, PVOID ptr, ULONG len, FILE_INFORMATION_CLASS class) switch (class) { ... default: FIXME("Unsupported class (%d)\n", class); 18=FileAllInformation called by dlls/ntdll/tests/file.c:1111 in test_file_all_information res = pNtSetInformationFile(h, &io, &fai_buf.fai, sizeof fai_buf, FileAllInformation); should not be 'FIXME'?
Now you come along and make loud demands that the patch be rejected, without having looked at the situation carefully. Frankly, this looks very much like the activities of an 'in-crowd' trying to defend its boarders.
You need to take it easy, man. No one is out to get you :)
A lot of folks on this list don't have English as a first language, and it can be easy to sound offensive if you haven't had the experience with the subtleties English that native speakers have had. There might also be some folks who are just abrasive, and you have to ignore or politely respond to those. In no case does accusing people like this help, even if they are doing some injustice towards you.
I don't think this has anything to do with me personally. I see it as pretty standard behaviour when an 'in-crowd' is chalanged.
Wine has a high barrier for entry and patches are reviewed harshly. If people are responding negatively to your patch, then it's likely because your patch was not obviously correct. The correct way to respond to this is by proving that it's correct, not asserting that it's correct. You're going to have to deal with defending your patches, and accept that sometimes you are wrong and sometimes other people are wrong.
Andrew
Yes, some people are abrasive. Giving them a pass only encourages their bad behavior. If Nicolay or anybody else was not happy with the pointer I gave to the test case, they could have simply said so.
Proof is necessarily in the mind of the reader. If the reader is not willing to examine the proof, they are impossible to convince. Similarly, if they are prejudiced, it will be impossible to change their minds no matter how much evidence is presented. On the other hand you asked for more specific information. I am providing it, but you will have to convince yourself that the arguments presented are correct.
If you will notice, I've already admitted to being wrong on some things. To some people that encourages their attack behavior.
- Max