crypt32: Remove a test because of a Windows 2003 SP1 bug

Reece Dunn msclrhd at googlemail.com
Thu Oct 25 17:49:04 CDT 2007


On 25/10/2007, Juan Lang <juan.lang at gmail.com> wrote:
> > I was basing it on what you said in the commit - both the heading and
> > the comment in the patch.
>
> That's dangerous here.  The details are small, but they matter:

Of course the details matter.

> >   1.  This patch is applied to remove the test for this feature^Wbug
> > in CertGetCertificateChain;
>
> No, the feature/bug is in CertGetIssuerCertificateFromStore.

Yeah, I copied the wrong function name. My mistake.

> And the
> test tested for a single bit being cleared.  In Windows 2003 SP1, that
> bit is instead set.  So leaving the test as-is is misleading:  it
> fails on at least one Windows platform (Windows 98 is unknown, because
> the crypt32 tests aren't running there for some reason that I don't
> know.)  Changing it to cover both possibilities is my usual tactic,
> but in the case of a single bit, there are only two possibilities.
> Hence the comment in the patch:  the bit being either set or cleared
> is acceptable on some system on which the test is run.  How, then, is
> the test meaningful?

Testing both cases would be silly in this instance, as you say in the
comment, as it would be like testing true or false is returned from a
function.

The only other thing would be for either Windows to agree (which it
doesn't), or there be some documentation to describe the correct
behaviour, or otherwise document the bug in one of the Windows
versions. Assuming that you haven't found any, there is no reference
to deduce what the result should be. Therefore, I withdraw my initial
objection.

> >   2.  Someone in the future submits a patch to fix the 'bug' in
> > CertGetCertificateChain for Wine;
>
> No - wrong function, and one which is not inconsistent in at least
> this detail on different versions of Windows.

I meant to reference the other function. My mistake.

> > Even if this is a deprecated API, there will still be applications
> > that are dependent on it.
>
> Of course, and the tests for the behavior of this API that is
> consistent remain.  I just remove the test of this single bit, and
> document why.

I retract my initial comments: the patch makes sense now.

- Reece



More information about the wine-devel mailing list