[PATCH 0/6] MR8337: server: Simplify atom table code.

[PATCH 1/6] server: Use a static array for atom table atoms.

[Rémi Bernon]

From: Rémi Bernon

---
 server/atom.c | 52 ++++++++++++++++++---------------------------------
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/server/atom.c b/server/atom.c
index ff0799f5880..d1a8fd6ae3e 100644
--- a/server/atom.c
+++ b/server/atom.c
@@ -61,13 +61,14 @@ struct atom_entry
 struct atom_table
 {
     struct object       obj;                 /* object header */
-    int                 count;               /* count of atom handles */
     int                 last;                /* last handle in-use */
-    struct atom_entry **handles;             /* atom handles */
+    struct atom_entry  *atoms[MAX_ATOMS];    /* atom entries */
     int                 entries_count;       /* number of hash entries */
     struct atom_entry **entries;             /* hash table entries */
 };

+C_ASSERT( sizeof(struct atom_table) <= 256 * 1024 );
+
 static void atom_table_dump( struct object *obj, int verbose );
 static void atom_table_destroy( struct object *obj );

@@ -106,18 +107,16 @@ static struct atom_table *create_table(int entries_count)
     {
         if ((entries_count < MIN_HASH_SIZE) ||
             (entries_count > MAX_HASH_SIZE)) entries_count = HASH_SIZE;
-        table->handles = NULL;
         table->entries_count = entries_count;
         if (!(table->entries = malloc( sizeof(*table->entries) * table->entries_count )))
         {
             set_error( STATUS_NO_MEMORY );
             goto fail;
         }
+        memset( table->atoms, 0, sizeof(*table->atoms) * ARRAY_SIZE(table->atoms) );
         memset( table->entries, 0, sizeof(*table->entries) * table->entries_count );
-        table->count = 64;
         table->last  = -1;
-        if ((table->handles = mem_alloc( sizeof(*table->handles) * table->count )))
-            return table;
+        return table;
 fail:
         release_object( table );
         table = NULL;
@@ -130,7 +129,7 @@ static struct atom_entry *get_atom_entry( struct atom_table *table, atom_t atom
 {
     struct atom_entry *entry = NULL;
     if (table && (atom >= MIN_STR_ATOM) && (atom <= MIN_STR_ATOM + table->last))
-        entry = table->handles[atom - MIN_STR_ATOM];
+        entry = table->atoms[atom - MIN_STR_ATOM];
     if (!entry) set_error( STATUS_INVALID_HANDLE );
     return entry;
 }
@@ -139,26 +138,11 @@ static struct atom_entry *get_atom_entry( struct atom_table *table, atom_t atom
 static atom_t add_atom_entry( struct atom_table *table, struct atom_entry *entry )
 {
     int i;
-    for (i = 0; i <= table->last; i++)
-        if (!table->handles[i]) goto found;
-    if (i == table->count)
-    {
-        struct atom_entry **new_table = NULL;
-        int new_size = table->count + table->count / 2;
-        if (new_size > MAX_ATOMS) new_size = MAX_ATOMS;
-        if (new_size > table->count)
-            new_table = realloc( table->handles, sizeof(*table->handles) * new_size );
-        if (!new_table)
-        {
-            set_error( STATUS_NO_MEMORY );
-            return 0;
-        }
-        table->count = new_size;
-        table->handles = new_table;
-    }
+    for (i = 0; i <= table->last; i++) if (!table->atoms[i]) goto found;
+    if (i == ARRAY_SIZE(table->atoms)) return 0;
     table->last = i;
  found:
-    table->handles[i] = entry;
+    table->atoms[i] = entry;
     entry->atom = i + MIN_STR_ATOM;
     return entry->atom;
 }
@@ -175,7 +159,7 @@ static void atom_table_dump( struct object *obj, int verbose )
     if (!verbose) return;
     for (i = 0; i <= table->last; i++)
     {
-        struct atom_entry *entry = table->handles[i];
+        struct atom_entry *entry = table->atoms[i];
         if (!entry) continue;
         fprintf( stderr, "  %04x: ref=%d pinned=%c hash=%d "",
                  entry->atom, entry->count, entry->pinned ? 'Y' : 'N', entry->hash );
@@ -190,11 +174,7 @@ static void atom_table_destroy( struct object *obj )
     int i;
     struct atom_table *table = (struct atom_table *)obj;
     assert( obj->ops == &atom_table_ops );
-    if (table->handles)
-    {
-        for (i = 0; i <= table->last; i++) free( table->handles[i] );
-        free( table->handles );
-    }
+    for (i = 0; i <= table->last; i++) free( table->atoms[i] );
     free( table->entries );
 }

@@ -247,9 +227,13 @@ static atom_t add_atom( struct atom_table *table, const struct unicode_str *str
             entry->len    = str->len;
             memcpy( entry->str, str->str, str->len );
         }
-        else free( entry );
+        else
+        {
+            set_error( STATUS_NO_MEMORY );
+            free( entry );
+        }
     }
-    else set_error( STATUS_NO_MEMORY );
+
     return atom;
 }

@@ -264,7 +248,7 @@ static void delete_atom( struct atom_table *table, atom_t atom, int if_pinned )
         if (entry->next) entry->next->prev = entry->prev;
         if (entry->prev) entry->prev->next = entry->next;
         else table->entries[entry->hash] = entry->next;
-        table->handles[atom - MIN_STR_ATOM] = NULL;
+        table->atoms[atom - MIN_STR_ATOM] = NULL;
         free( entry );
     }
 }

[PATCH 2/6] server: Use a count instead of last atom index.

[Rémi Bernon]

From: Rémi Bernon

---
 server/atom.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/server/atom.c b/server/atom.c
index d1a8fd6ae3e..7c58f286b31 100644
--- a/server/atom.c
+++ b/server/atom.c
@@ -61,7 +61,7 @@ struct atom_entry
 struct atom_table
 {
     struct object       obj;                 /* object header */
-    int                 last;                /* last handle in-use */
+    int                 count;               /* number of used atoms */
     struct atom_entry  *atoms[MAX_ATOMS];    /* atom entries */
     int                 entries_count;       /* number of hash entries */
     struct atom_entry **entries;             /* hash table entries */
@@ -115,7 +115,7 @@ static struct atom_table *create_table(int entries_count)
         }
         memset( table->atoms, 0, sizeof(*table->atoms) * ARRAY_SIZE(table->atoms) );
         memset( table->entries, 0, sizeof(*table->entries) * table->entries_count );
-        table->last  = -1;
+        table->count = 0;
         return table;
 fail:
         release_object( table );
@@ -128,7 +128,7 @@ fail:
 static struct atom_entry *get_atom_entry( struct atom_table *table, atom_t atom )
 {
     struct atom_entry *entry = NULL;
-    if (table && (atom >= MIN_STR_ATOM) && (atom <= MIN_STR_ATOM + table->last))
+    if (table && (atom >= MIN_STR_ATOM) && (atom < MIN_STR_ATOM + table->count))
         entry = table->atoms[atom - MIN_STR_ATOM];
     if (!entry) set_error( STATUS_INVALID_HANDLE );
     return entry;
@@ -138,10 +138,9 @@ static struct atom_entry *get_atom_entry( struct atom_table *table, atom_t atom
 static atom_t add_atom_entry( struct atom_table *table, struct atom_entry *entry )
 {
     int i;
-    for (i = 0; i <= table->last; i++) if (!table->atoms[i]) goto found;
+    for (i = 0; i < table->count; i++) if (!table->atoms[i]) break;
     if (i == ARRAY_SIZE(table->atoms)) return 0;
-    table->last = i;
- found:
+    if (i == table->count) table->count++;
     table->atoms[i] = entry;
     entry->atom = i + MIN_STR_ATOM;
     return entry->atom;
@@ -154,10 +153,9 @@ static void atom_table_dump( struct object *obj, int verbose )
     struct atom_table *table = (struct atom_table *)obj;
     assert( obj->ops == &atom_table_ops );

-    fprintf( stderr, "Atom table size=%d entries=%d\n",
-             table->last + 1, table->entries_count );
+    fprintf( stderr, "Atom table size=%d entries=%d\n", table->count, table->entries_count );
     if (!verbose) return;
-    for (i = 0; i <= table->last; i++)
+    for (i = 0; i < table->count; i++)
     {
         struct atom_entry *entry = table->atoms[i];
         if (!entry) continue;
@@ -174,7 +172,7 @@ static void atom_table_destroy( struct object *obj )
     int i;
     struct atom_table *table = (struct atom_table *)obj;
     assert( obj->ops == &atom_table_ops );
-    for (i = 0; i <= table->last; i++) free( table->atoms[i] );
+    for (i = 0; i < table->count; i++) free( table->atoms[i] );
     free( table->entries );
 }

[PATCH 3/6] server: Forbid using string atom 0xc000.

[Rémi Bernon]

From: Rémi Bernon

---
 dlls/win32u/tests/win32u.c | 16 ++++++++++++++--
 server/atom.c              |  4 ++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/dlls/win32u/tests/win32u.c b/dlls/win32u/tests/win32u.c
index acbc5bc3034..ac044c30f83 100644
--- a/dlls/win32u/tests/win32u.c
+++ b/dlls/win32u/tests/win32u.c
@@ -341,6 +341,17 @@ static void test_class(void)
         "NtUserGetAtomName returned %lx %lu\n", ret, GetLastError() );
     ok( buf[0] == 0xcccc, "buf = %s\n", debugstr_w(buf) );

+    status = NtQueryInformationAtom( 0xc000, AtomBasicInformation, abi, sizeof(abi_buf), NULL );
+    ok( status == STATUS_INVALID_HANDLE, "NtQueryInformationAtom returned %#lx\n", status );
+
+    memset( buf, 0xcc, sizeof(buf) );
+    name.Buffer = buf;
+    name.Length = 0xdead;
+    name.MaximumLength = sizeof(buf);
+    ret = NtUserGetAtomName( 0xc000, &name );
+    ok( !ret && GetLastError() == ERROR_INVALID_HANDLE,
+        "NtUserGetAtomName returned %lx %lu\n", ret, GetLastError() );
+
     for (int i = 0; i < ARRAY_SIZE(global_atoms); i++)
     {
         winetest_push_context( "%#x: %s", global_atoms[i].atom, debugstr_w(global_atoms[i].name) );
@@ -354,6 +365,7 @@ static void test_class(void)
         name.Length = 0xdead;
         name.MaximumLength = sizeof(buf);
         ret = NtUserGetAtomName( global_atoms[i].atom, &name );
+        todo_wine_if( i == 0 || i == 8 || i == 11 )
         ok( ret != wcslen( global_atoms[i].name ), "NtUserGetAtomName returned %lu\n", ret );
         ok( name.Length == 0xdead, "Length = %u\n", name.Length );
         ok( name.MaximumLength == sizeof(buf), "MaximumLength = %u\n", name.MaximumLength );
@@ -381,7 +393,7 @@ static void test_class(void)
             win_skip( "Skipping user atoms check on W11\n" );
             break;
         }
-        todo_wine_if( i != 0 && i != 2 && i != 6 )
+        todo_wine_if( i != 6 && i != 12 )
         ok( ret == wcslen( user_atoms[i].name ), "NtUserGetAtomName returned %lu\n", ret );
         ok( name.Length == 0xdead, "Length = %u\n", name.Length );
         ok( name.MaximumLength == sizeof(buf), "MaximumLength = %u\n", name.MaximumLength );
@@ -392,7 +404,7 @@ static void test_class(void)
                               CW_USEDEFAULT, 0, CW_USEDEFAULT, 0, 0, 0, NULL, 0 );
         if (!user_atoms[i].class)
         {
-            todo_wine_if( i < 9 ) ok( !hwnd, "CreateWindowW succeeded\n" );
+            todo_wine_if( i < 10 ) ok( !hwnd, "CreateWindowW succeeded\n" );
             todo_wine ok( GetLastError() == ERROR_CANNOT_FIND_WND_CLASS, "got error %lu\n", GetLastError() );
             if (hwnd) DestroyWindow( hwnd );
         }
diff --git a/server/atom.c b/server/atom.c
index 7c58f286b31..70e454d3cc3 100644
--- a/server/atom.c
+++ b/server/atom.c
@@ -115,7 +115,7 @@ static struct atom_table *create_table(int entries_count)
         }
         memset( table->atoms, 0, sizeof(*table->atoms) * ARRAY_SIZE(table->atoms) );
         memset( table->entries, 0, sizeof(*table->entries) * table->entries_count );
-        table->count = 0;
+        table->count = 1; /* atom 0xc000 is reserved */
         return table;
 fail:
         release_object( table );
@@ -138,7 +138,7 @@ static struct atom_entry *get_atom_entry( struct atom_table *table, atom_t atom
 static atom_t add_atom_entry( struct atom_table *table, struct atom_entry *entry )
 {
     int i;
-    for (i = 0; i < table->count; i++) if (!table->atoms[i]) break;
+    for (i = 1 /* atom 0xc000 is reserved */; i < table->count; i++) if (!table->atoms[i]) break;
     if (i == ARRAY_SIZE(table->atoms)) return 0;
     if (i == table->count) table->count++;
     table->atoms[i] = entry;

[PATCH 4/6] server: Use a static array for atom table hash.

[Rémi Bernon]

From: Rémi Bernon

---
 server/atom.c | 39 ++++++++++++---------------------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/server/atom.c b/server/atom.c
index 70e454d3cc3..6f1510ed99b 100644
--- a/server/atom.c
+++ b/server/atom.c
@@ -63,8 +63,7 @@ struct atom_table
     struct object       obj;                 /* object header */
     int                 count;               /* number of used atoms */
     struct atom_entry  *atoms[MAX_ATOMS];    /* atom entries */
-    int                 entries_count;       /* number of hash entries */
-    struct atom_entry **entries;             /* hash table entries */
+    struct atom_entry  *entries[HASH_SIZE];  /* hash table entries */
 };

C_ASSERT( sizeof(struct atom_table) <= 256 * 1024 );
@@ -99,28 +98,15 @@ static const struct object_ops atom_table_ops =
 static struct atom_table *global_table;

/* create an atom table */
-static struct atom_table *create_table(int entries_count)
+static struct atom_table *create_table(void)
 {
     struct atom_table *table;

-    if ((table = alloc_object( &atom_table_ops )))
-    {
-        if ((entries_count < MIN_HASH_SIZE) ||
-            (entries_count > MAX_HASH_SIZE)) entries_count = HASH_SIZE;
-        table->entries_count = entries_count;
-        if (!(table->entries = malloc( sizeof(*table->entries) * table->entries_count )))
-        {
-            set_error( STATUS_NO_MEMORY );
-            goto fail;
-        }
-        memset( table->atoms, 0, sizeof(*table->atoms) * ARRAY_SIZE(table->atoms) );
-        memset( table->entries, 0, sizeof(*table->entries) * table->entries_count );
-        table->count = 1; /* atom 0xc000 is reserved */
-        return table;
-fail:
-        release_object( table );
-        table = NULL;
-    }
+    if (!(table = alloc_object( &atom_table_ops ))) return NULL;
+    memset( table->atoms, 0, sizeof(*table->atoms) * ARRAY_SIZE(table->atoms) );
+    memset( table->entries, 0, sizeof(*table->entries) * ARRAY_SIZE(table->entries) );
+    table->count = 1; /* atom 0xc000 is reserved */
+
     return table;
 }

@@ -153,7 +139,7 @@ static void atom_table_dump( struct object *obj, int verbose )
     struct atom_table *table = (struct atom_table *)obj;
     assert( obj->ops == &atom_table_ops );

-    fprintf( stderr, "Atom table size=%d entries=%d\n", table->count, table->entries_count );
+    fprintf( stderr, "Atom table size=%d\n", table->count );
     if (!verbose) return;
     for (i = 0; i < table->count; i++)
     {
@@ -173,7 +159,6 @@ static void atom_table_destroy( struct object *obj )
     struct atom_table *table = (struct atom_table *)obj;
     assert( obj->ops == &atom_table_ops );
     for (i = 0; i < table->count; i++) free( table->atoms[i] );
-    free( table->entries );
 }

/* find an atom entry in its hash list */
@@ -193,7 +178,7 @@ static struct atom_entry *find_atom_entry( struct atom_table *table, const struc
 static atom_t add_atom( struct atom_table *table, const struct unicode_str *str )
 {
     struct atom_entry *entry;
-    unsigned short hash = hash_strW( str->str, str->len, table->entries_count );
+    unsigned short hash = hash_strW( str->str, str->len, ARRAY_SIZE(table->entries) );
     atom_t atom = 0;

if (!str->len)
@@ -267,7 +252,7 @@ static atom_t find_atom( struct atom_table *table, const struct unicode_str *str
         return 0;
     }
     if (table && (entry = find_atom_entry( table, str,
-                                           hash_strW( str->str, str->len, table->entries_count ))))
+                                           hash_strW( str->str, str->len, ARRAY_SIZE(table->entries) ))))
         return entry->atom;
     set_error( STATUS_OBJECT_NAME_NOT_FOUND );
     return 0;
@@ -280,7 +265,7 @@ static struct atom_table *get_global_table( struct winstation *winstation, int c
     {
         if (create)
         {
-            table = create_table( HASH_SIZE );
+            table = create_table();
             if (winstation) winstation->atom_table = table;
             else
             {
@@ -308,7 +293,7 @@ atom_t find_global_atom( struct winstation *winstation, const struct unicode_str
     struct atom_entry *entry;

if (!str->len || str->len > MAX_ATOM_LEN || !table) return 0;
-    if ((entry = find_atom_entry( table, str, hash_strW( str->str, str->len, table->entries_count ))))
+    if ((entry = find_atom_entry( table, str, hash_strW( str->str, str->len, ARRAY_SIZE(table->entries) ))))
         return entry->atom;
     return 0;
 }

[PATCH 5/6] server: Keep computed atom hash in local variables.

[Rémi Bernon]

From: Rémi Bernon

---
 server/atom.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/server/atom.c b/server/atom.c
index 6f1510ed99b..a22e511a4aa 100644
--- a/server/atom.c
+++ b/server/atom.c
@@ -178,7 +178,7 @@ static struct atom_entry *find_atom_entry( struct atom_table *table, const struc
 static atom_t add_atom( struct atom_table *table, const struct unicode_str *str )
 {
     struct atom_entry *entry;
-    unsigned short hash = hash_strW( str->str, str->len, ARRAY_SIZE(table->entries) );
+    unsigned short hash;
     atom_t atom = 0;

if (!str->len)
@@ -191,6 +191,8 @@ static atom_t add_atom( struct atom_table *table, const struct unicode_str *str
         set_error( STATUS_INVALID_PARAMETER );
         return 0;
     }
+
+    hash = hash_strW( str->str, str->len, ARRAY_SIZE(table->entries) );
     if ((entry = find_atom_entry( table, str, hash )))  /* exists already */
     {
         entry->count++;
@@ -240,6 +242,7 @@ static void delete_atom( struct atom_table *table, atom_t atom, int if_pinned )
 static atom_t find_atom( struct atom_table *table, const struct unicode_str *str )
 {
     struct atom_entry *entry;
+    unsigned short hash;

if (!str->len)
     {
@@ -251,11 +254,14 @@ static atom_t find_atom( struct atom_table *table, const struct unicode_str *str
         set_error( STATUS_INVALID_PARAMETER );
         return 0;
     }
-    if (table && (entry = find_atom_entry( table, str,
-                                           hash_strW( str->str, str->len, ARRAY_SIZE(table->entries) ))))
-        return entry->atom;
-    set_error( STATUS_OBJECT_NAME_NOT_FOUND );
-    return 0;
+
+    hash = hash_strW( str->str, str->len, ARRAY_SIZE(table->entries) );
+    if (!(entry = find_atom_entry( table, str, hash )))
+    {
+        set_error( STATUS_OBJECT_NAME_NOT_FOUND );
+        return 0;
+    }
+    return entry->atom;
 }

static struct atom_table *get_global_table( struct winstation *winstation, int create )
@@ -291,11 +297,13 @@ atom_t find_global_atom( struct winstation *winstation, const struct unicode_str
 {
     struct atom_table *table = get_global_table( winstation, 0 );
     struct atom_entry *entry;
+    unsigned short hash;

if (!str->len || str->len > MAX_ATOM_LEN || !table) return 0;
-    if ((entry = find_atom_entry( table, str, hash_strW( str->str, str->len, ARRAY_SIZE(table->entries) ))))
-        return entry->atom;
-    return 0;
+
+    hash = hash_strW( str->str, str->len, ARRAY_SIZE(table->entries) );
+    if (!(entry = find_atom_entry( table, str, hash ))) return 0;
+    return entry->atom;
 }

/* increment the ref count of a global atom; used for window properties */

[PATCH 6/6] server: Remove unused atom pinned member.

[Rémi Bernon]

From: Rémi Bernon

---
 server/atom.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/server/atom.c b/server/atom.c
index a22e511a4aa..d7a4af9ab5f 100644
--- a/server/atom.c
+++ b/server/atom.c
@@ -51,7 +51,6 @@ struct atom_entry
     struct atom_entry *next;   /* hash table list */
     struct atom_entry *prev;   /* hash table list */
     int                count;  /* reference count */
-    short              pinned; /* whether the atom is pinned or not */
     atom_t             atom;   /* atom handle */
     unsigned short     hash;   /* string hash */
     unsigned short     len;    /* string len */
@@ -145,8 +144,8 @@ static void atom_table_dump( struct object *obj, int verbose )
     {
         struct atom_entry *entry = table->atoms[i];
         if (!entry) continue;
-        fprintf( stderr, "  %04x: ref=%d pinned=%c hash=%d "",
-                 entry->atom, entry->count, entry->pinned ? 'Y' : 'N', entry->hash );
+        fprintf( stderr, "  %04x: ref=%d hash=%d "",
+                 entry->atom, entry->count, entry->hash );
         dump_strW( entry->str, entry->len, stderr, """");
         fprintf( stderr, ""\n" );
     }
@@ -207,7 +206,6 @@ static atom_t add_atom( struct atom_table *table, const struct unicode_str *str
             if ((entry->next = table->entries[hash])) entry->next->prev = entry;
             table->entries[hash] = entry;
             entry->count  = 1;
-            entry->pinned = 0;
             entry->hash   = hash;
             entry->len    = str->len;
             memcpy( entry->str, str->str, str->len );
@@ -227,8 +225,7 @@ static void delete_atom( struct atom_table *table, atom_t atom, int if_pinned )
 {
     struct atom_entry *entry = get_atom_entry( table, atom );
     if (!entry) return;
-    if (entry->pinned && !if_pinned) set_error( STATUS_WAS_LOCKED );
-    else if (!--entry->count)
+    if (!--entry->count)
     {
         if (entry->next) entry->next->prev = entry->prev;
         if (entry->prev) entry->prev->next = entry->next;
@@ -372,7 +369,7 @@ DECL_HANDLER(get_atom_information)
         {
             set_reply_data( entry->str, min( entry->len, get_reply_max_size() ));
             reply->count = entry->count;
-            reply->pinned = entry->pinned;
+            reply->pinned = 0;
             reply->total = entry->len;
         }
         else reply->count = -1;

[PATCH 0/6] MR8337: Draft: server: Simplify atom table code.

[Rémi Bernon (＠rbernon)]

Sorry, this will obviously conflict with https://gitlab.winehq.org/wine/wine/-/merge_requests/8325, I'll update it later.