New subject: kernel32: Don't set unconditionally SECTION_QUERY flag in OpenFileMapping.

11 Mar 2016

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@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.


    

Re: kernel32: Don't set unconditionally SECTION_QUERY flag in OpenFileMapping.