Re: [PATCH 1/2] advapi32: HKCR merge: foundation

[George Stephanos]

On Fri, Sep 27, 2013 at 11:13 AM, Alexandre Julliard wrote:
> George Stephanos  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
> [e-mail]