Thanks, I have only style related comments, and since I'm an author of existing tests I'd prefer to keep an original style if possible. Jacek Caban <jacek(a)codeweavers.com> wrote:
--- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -5560,17 +5560,23 @@ static void test_filemap_security(void) char temp_path[MAX_PATH]; char file_name[MAX_PATH]; DWORD ret, i, access; - HANDLE file, mapping, dup; + HANDLE file, mapping, dup, created_mapping; static const struct { - int generic, mapped; + DWORD generic, mapped;
Please leave 'int' type, it changes nothing.
+ BOOL open_only; } map[] = { { 0, 0 }, { GENERIC_READ, STANDARD_RIGHTS_READ | SECTION_QUERY | SECTION_MAP_READ }, { GENERIC_WRITE, STANDARD_RIGHTS_WRITE | SECTION_MAP_WRITE }, { GENERIC_EXECUTE, STANDARD_RIGHTS_EXECUTE | SECTION_MAP_EXECUTE }, - { GENERIC_ALL, STANDARD_RIGHTS_REQUIRED | SECTION_ALL_ACCESS } + { GENERIC_ALL, STANDARD_RIGHTS_REQUIRED | SECTION_ALL_ACCESS }, + { SECTION_MAP_READ | SECTION_MAP_WRITE, SECTION_MAP_READ | SECTION_MAP_WRITE}, + { SECTION_MAP_WRITE, SECTION_MAP_WRITE}, + { SECTION_MAP_READ | SECTION_QUERY, SECTION_MAP_READ | SECTION_QUERY}, + { SECTION_QUERY, SECTION_MAP_READ, TRUE }, + { SECTION_QUERY | SECTION_MAP_READ, SECTION_QUERY | SECTION_MAP_READ } }; static const struct { @@ -5599,6 +5605,8 @@ static void test_filemap_security(void)
for (i = 0; i < sizeof(prot_map)/sizeof(prot_map[0]); i++) { + if (map[i].open_only) continue; + SetLastError(0xdeadbeef); mapping = CreateFileMappingW(file, NULL, prot_map[i].prot, 0, 4096, NULL); if (prot_map[i].mapped) @@ -5645,6 +5653,8 @@ static void test_filemap_security(void)
for (i = 0; i < sizeof(map)/sizeof(map[0]); i++) { + if (map[i].open_only) continue; + SetLastError( 0xdeadbeef ); ret = DuplicateHandle(GetCurrentProcess(), mapping, GetCurrentProcess(), &dup, map[i].generic, FALSE, 0); @@ -5659,6 +5669,24 @@ static void test_filemap_security(void) CloseHandle(mapping); CloseHandle(file); DeleteFileA(file_name); + + created_mapping = CreateFileMappingA( INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, 0x1000, + "Wine Test Open Mapping"); + ok( created_mapping != NULL, "CreateFileMapping failed with error %u\n", GetLastError() ); + + for(i=0; i < sizeof(map)/sizeof(*map); i++)
Please add spaces around '=' and use sizeof(map[0]) for consistency with existing code.
+ { + if (!map[i].generic) continue; + + mapping = OpenFileMappingA( map[i].generic, FALSE, "Wine Test Open Mapping"); + ok( mapping != NULL, "OpenFileMapping failed with error %d\n", GetLastError()); + access = get_obj_access( mapping ); + ok( access == map[i].mapped, "[%d] unexpected access flags %x, expected %x\n", + i, access, map[i].mapped );
Please use "%d:" instead of [%d] and %#x instead of %x for consistency with existing code. P.S. That's perfectly fine to completely ignore my comments, just wanted to show how it looks like for comparison with d3d-like acceptance policy :) -- Dmitry.