On Fri, Sep 27, 2013 at 11:13 AM, Alexandre Julliard <julliard@winehq.org> wrote:

George Stephanos <gaf.stephanos@gmail.com> writes:

> @@ -224,13 +248,138 @@ static NTSTATUS open_key( HKEY *retkey, ACCESS_MASK access, OBJECT_ATTRIBUTES *a
>
> }
>
> +static LSTATUS create_hkcr_struct( HKEY *hkey, opened_hkcr_t **hkcr )
> +{
> + UINT_PTR handle = nb_hkcr_handles, i;
> + LSTATUS ret = ERROR_SUCCESS;
> +
> + EnterCriticalSection( &hkcr_handles_cs );
> +
> + for (i = 0; i < nb_hkcr_handles; i++)
> + {
> + if (!hkcr_handles[i])
> + {
> + handle = i;
> + break;
> + }
> + }

You need some sort of free list instead of a linear search.

Alright.

Also you can probably avoid one level of pointers and store objects directly.

I thought about this. If I store objects directly, accessing any would require a lock on the whole table so I guarantee it's not moved or reallocated elsewhere. This would obviously be pretty slow.

Returning a pointer to the object outside of the critical section is not
a good idea.

The critical section just protects the table and not the handles/structs themselves.

> +static HKEY resolve_hkcr( HKEY hkey )
> +{
> + HKEY ret;
> + UINT_PTR idx = (UINT_PTR)hkey >> 2;
> + opened_hkcr_t *hkcr;
> + if (idx <= nb_hkcr_handles)
> + {
> + EnterCriticalSection( &hkcr_handles_cs );

The count needs to be protected by the critical section too.

Also please write a better subject line for the commit.

--
Alexandre Julliard
julliard@winehq.org