[PATCH] dlls/ntdll/file.c: Setting FileAllInformation is not 'fixable'.

[Andrew Eikum]

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?

> 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.

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