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

[Max TenEyck Woodbury]

On 07/25/2010 12:50 AM, Dmitry Timoshkov wrote:
> Max TenEyck Woodbury  wrote:
> > > > - /* Invalid requests - do not need 'fixing'. */
> > > > - case FileAllInformation:
> > > > - io->u.Status = STATUS_NOT_IMPLEMENTED;
> > > > - break;
> > > > -
> > > >
> > > > default:
> > > > FIXME("Unsupported class (%d)\n", class);
> > > > io->u.Status = STATUS_NOT_IMPLEMENTED;
> > >
> > > Add a test please, and a comment won't be needed with a test too.
> >
> > There already is a test in dlls/ntdll/test/file.c. It produces a
> > 'fixme:' when it should not. This fixes that.
>
> This is not a fix. A fix is a patch that makes a test pass that previously
> did not. You simply silence a harmless fixme.

The 'fixme' is not 'harmless'. It gets counted. It claims that there is
something wrong that needs fixing. In fact, the proper implementation
for this particular request is to set an error code and ignore the
request. The test code checks for exactly that condition and does NOT 
report a test failure when that happens. It reports a failure if the 
error code does NOT have the specified value. In other words this patch
fixes an error in the 'work to be done count'.

Please stop the sniping. There is now more text disparaging the patch
than there is text in the patch itself.

-Max