[PATCH 1/5] lz32/tests: Test more last errors set by LZOpenFile[AW].

[Saulius Krasuckas]

* On Thu, 5 Oct 2006, Vitaliy Margolen wrote:
> * Saulius Krasuckas wrote:
> > --- a/dlls/lz32/tests/lzexpand_main.c
> > +++ b/dlls/lz32/tests/lzexpand_main.c
> > @@ -131,6 +131,7 @@ static void test_LZOpenFileA_existing_co
> >    memset(&filled_0xA5, 0xA5, OFS_MAXPATHNAME);
> >    memset(&test, 0xA5, sizeof(test));
> >    full_file_path_name_in_a_CWD(filename_, expected, FALSE);
> > +  SetLastError(0xfaceabee);
> 
> Please use the common accepted and agreed on 0xdeadbeef. As it is used 
> in all the other tests.

I am all in a big wait for some text page, telling about the need to clean 
non-dead-beefed SetLastError calls for example using some chained greps:

$ grep -Iri 'SetLastError[^(]*([^_a-z0-9]*[_a-z0-9]\+[^_a-z0-9]*)' dlls/*/tests/*.c 
  | grep -v '#.*define' 
  | grep -vE '\([^a-z]*(0xdeadbeef|ERROR_SUCCESS|0)[^a-z]*\)'

output of which gives a number 180 (when routed via "wc -l") and following 
list (when routed via "sort | uniq"):

dlls/advapi32/tests/crypt.c:			SetLastError(NTE_PROV_TYPE_ENTRY_BAD);
dlls/advapi32/tests/security.c:    SetLastError(NO_ERROR);
dlls/crypt32/tests/cert.c:        SetLastError(0xbaadcafe);
dlls/crypt32/tests/protectdata.c:    SetLastError(0xDEADBEEF);
dlls/kernel32/tests/alloc.c:        SetLastError(NO_ERROR);
dlls/kernel32/tests/alloc.c:    SetLastError(NO_ERROR);
dlls/kernel32/tests/change.c:    SetLastError(0xd0b00b00);
dlls/kernel32/tests/file.c:    SetLastError( 0xb00 );
dlls/kernel32/tests/file.c:    SetLastError( 0xdeadbeaf );
dlls/kernel32/tests/file.c:        SetLastError(0xfaceabee);
dlls/kernel32/tests/file.c:    SetLastError(0xfaceabee);
dlls/kernel32/tests/file.c:    SetLastError(12345678);
dlls/kernel32/tests/heap.c:    SetLastError(MAGIC_DEAD);
dlls/kernel32/tests/thread.c:  SetLastError(0xFACEaBAD);
dlls/lz32/tests/lzexpand_main.c:  SetLastError(0xfaceabee);
dlls/mscms/tests/profile.c:    SetLastError(0xfaceabee); /* 1st, maybe 2nd and then dereferenced 4th param, */
dlls/mscms/tests/profile.c:    SetLastError(0xfaceabee); /* 1st param, */
dlls/mscms/tests/profile.c:    SetLastError(0xfaceabee); /* 2nd param, */
dlls/mscms/tests/profile.c:        SetLastError(0xfaceabee); /* 3rd param, */
dlls/mscms/tests/profile.c:    SetLastError(0xfaceabee); /* 3th param, */
dlls/mscms/tests/profile.c:    SetLastError(0xfaceabee); /* 4th param, */
dlls/mscms/tests/profile.c:        SetLastError(0xfaceabee); /* dereferenced 4th param, */
dlls/mscms/tests/profile.c:        SetLastError(0xfaceabee); /* dereferenced 4th param. */
dlls/mscms/tests/profile.c:    SetLastError(0xfaceabee); /* dereferenced 4th param. */
dlls/mscms/tests/profile.c:    SetLastError(0xfaceabee); /* maybe 2nd and then 4th param, */
dlls/mscms/tests/profile.c:    SetLastError(0xfaceabee); /* maybe 2nd param. */
dlls/mscms/tests/profile.c:    SetLastError(0xfaceabee); /* maybe 2nd, then 3rd and dereferenced 4th param, */
dlls/setupapi/tests/query.c:    SetLastError(0xbeefcafe);
dlls/shell32/tests/shlexec.c:    SetLastError(0xcafebabe);
dlls/version/tests/info.c:    SetLastError(MY_LAST_ERROR);
dlls/version/tests/info.c:	    SetLastError(MY_LAST_ERROR);
dlls/version/tests/info.c:	SetLastError(MY_LAST_ERROR);
dlls/wininet/tests/url.c:    SetLastError(0xfaceabad);
dlls/winspool.drv/tests/info.c:        SetLastError(MAGIC_DEAD);
dlls/winspool.drv/tests/info.c:    SetLastError(MAGIC_DEAD);

Did I persuade you, Vitaliy, or you, Dmitry?  Feel free to enhance my 
composite monster.  

If no such text is put somewhere near Wine docs, I wouldn't believe this 
requirement is deadly significant.  

Unless Alexandre confirms the requirement directly.  In such case I am 
going to put this stuff into our Wiki ^^ even write a patch for cleaning 
the calls myself ;)

> >    /* d, using underscore-terminated file name. */
> >    file = LZOpenFileA(_terminated, &test, OF_EXIST);
> >    ok(file >= 0, "LZOpenFileA failed on switching to a compressed file name\n");
> > +  ok(GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_BAD_PATHNAME,
> > +     "GetLastError() returns %ld\n", GetLastError());
> 
> This doesn't look right. It either fails or it doesn't. You can't have 
> both. Or this test is nor correct and should be removed.

Win32 app can easily stuck upon this if being run on WinME and WinXP.  
The result may be used to distinguish between 9x and NT platforms (just 
like we are doing in our tests to avoid calling GetVersion;), 

That was the reason I kept the check in.  It doesn't enforce unnecessary 
complexity in the function, so I doubt the check is wrong.  Especially 
given that this fn on both platforms behaves *quite* differently while 
operating on compressed file(name)s.

If you don't agree, please also comment on this output:

$ grep -IriC3 'ERROR_SUCCESS.*||.*GetLastError' dlls/*/tests/*.c 
 dlls/kernel32/tests/heap.c-    SetLastError(MAGIC_DEAD);
 dlls/kernel32/tests/heap.c-    res = GlobalUnlock(gbl);    /* #0 */
 dlls/kernel32/tests/heap.c-    /* NT: ERROR_SUCCESS (documented on MSDN), 9x: untouched */
 dlls/kernel32/tests/heap.c:    ok(!res && ((GetLastError() == ERROR_SUCCESS) || (GetLastError() == MAGIC_DEAD)),
 dlls/kernel32/tests/heap.c-        "returned %ld with %ld (expected '0' with: ERROR_SUCCESS or " \
 dlls/kernel32/tests/heap.c-        "MAGIC_DEAD)\n", res, GetLastError());
 dlls/kernel32/tests/heap.c-    SetLastError(MAGIC_DEAD);
 --
 dlls/kernel32/tests/heap.c-    SetLastError(MAGIC_DEAD);
 dlls/kernel32/tests/heap.c-    res = LocalUnlock(gbl);    /* #0 */
 dlls/kernel32/tests/heap.c-    /* NT: ERROR_SUCCESS (documented on MSDN), 9x: untouched */
 dlls/kernel32/tests/heap.c:    ok(!res && ((GetLastError() == ERROR_SUCCESS) || (GetLastError() == MAGIC_DEAD)),
 dlls/kernel32/tests/heap.c-        "returned %ld with %ld (expected '0' with: ERROR_SUCCESS or " \
 dlls/kernel32/tests/heap.c-        "MAGIC_DEAD)\n", res, GetLastError());
 dlls/kernel32/tests/heap.c-    SetLastError(MAGIC_DEAD);
 --
 dlls/shlwapi/tests/ordinal.c-    switch(retval) {
 dlls/shlwapi/tests/ordinal.c-   case 0L:
 dlls/shlwapi/tests/ordinal.c-            if(buffersize == exactsize) {
 dlls/shlwapi/tests/ordinal.c:            ok( (ERROR_SUCCESS == GetLastError()) || (ERROR_CALL_NOT_IMPLEMENTED == GetLastError()) ||
 dlls/shlwapi/tests/ordinal.c-           (ERROR_PROC_NOT_FOUND == GetLastError()) || (ERROR_NO_IMPERSONATION_TOKEN == GetLastError()),
 dlls/shlwapi/tests/ordinal.c-                "last error wrong: got %08lx; expected ERROR_SUCCESS(NT4)/ERROR_CALL_NOT_IMPLEMENTED(98/ME)/"
 dlls/shlwapi/tests/ordinal.c-           "ERROR_PROC_NOT_FOUND(NT4)/ERROR_NO_IMPERSONATION_TOKEN(XP)\n", GetLastError());

Vitaliy, do you believe the GLE checks in last chunk should be removed too?