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

[George Stephanos]

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

> George Stephanos <gaf.stephanos at 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 at winehq.org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20131025/4ea5f75f/attachment.html>