[PATCH v2] shlwapi/tests: Document where SHFormatDateTimeA() crashes for NULL pointers.

[Francois Gouget]

On Wed, 1 Sep 2021, Huw Davies wrote:
[...]
> I don't see the point of this sort of thing.  Why do we care that
> Windows crashes?  No sane app is ever going to rely on this.

I disagree with your statement "No sane app is ever going to rely on 
this".

For instance GdipGetFamilyName() used to crash when given a NULL output 
buffer (ad663360fae6). But that changed with Windows 7; 12 years ago. So 
I'd argue that it's now reasonable for developers to assume 
GdipGetFamilyName() does not crash in that case.

The test says that SHFormatDateTimeA(NULL...) "crashes on native". But 
_if_ that only describes the behavior of "Windows Vista or older" I'd 
argue that it's likewise reasonable for current applications to rely on 
the Windows 7+ behavior.

Now it turns out SHFormatDateTimeA() crashes even in the latest Windows 
versions so I'm just documenting that.

Also what is really tested is whether a NULL input pointer is allowed 
rather than a NULL output buffer. That may be less relevant, and could 
justify removing the test entirely.


> Documenting which Windows versions crash seems even more of a waste of
> time;

I'm not going to go through all the test in Wine to update them if 
that's what you're afraid of. But when I try to understand how an API 
works in order to fix a test (here a shlwapi:ordinal failure in 
Japanese) and I see such a badly documented test I'll certainly check it 
out.

Also any patch that contains an "if (0) /* crashes */" should have 
better documentation before it gets applied.


> are we really going to update this after every new Windows
> release?

I'm fine with removing the test if you prefer that.
But if it's going to stay it should have some indication of which 
Windows version crashed.


-- 
Francois Gouget <fgouget at codeweavers.com>