Re: [PATCH v4 1/5] compobj.dll16: Implement CoLoadLibrary16 and CoFreeLibrary16.

[Alexandre Julliard]

Zebediah Figura  writes:
> +/* --- loaded dll list implementation */
> +struct open_dll
> +{
> - HMODULE16 library;
> - FARPROC16 DllCanUnloadNow;
> - struct list entry;
>
> +};
> -
>
> +static struct list open_dll_list = LIST_INIT(open_dll_list);
> -
>
> +static HRESULT dll_list_add(LPCSTR library_name, struct open_dll **ret)
> +{
> - struct open_dll *dll;
> - HMODULE16 library;
> - FARPROC16 DllCanUnloadNow;
> -
> - library = LoadLibrary16(library_name);
> - if (!library)
> - {
> -
>    ERR("couldn't load in-process dll %s\n", debugstr_a(library_name));
> -
>    return E_ACCESSDENIED; /* FIXME: or should this be CO_E_DLLNOTFOUND? */
> - }
> -
> - DllCanUnloadNow = GetProcAddress16(library, "DllCanUnloadNow");
> - /* Note: failing to find DllCanUnloadNow is not a failure */
> -
> - dll = HeapAlloc(GetProcessHeap(), 0, sizeof(struct open_dll));
> - dll->library = library;
> - dll->DllCanUnloadNow = DllCanUnloadNow;
> - list_add_tail(&open_dll_list, &dll->entry);
> - *ret = dll;
> - TRACE("added new loaded dll %d\n", dll->library);
> -
> - return S_OK;
>
> +}

I'm not sure we even need the list at all. It would be needed for
CoFreeAllLibraries16, but it's not clear that this is a useful function.
> +void WINAPI CoFreeLibrary16(
> - HINSTANCE16 hInst)
>
> +{
> - struct open_dll *dll;
> -
> - LIST_FOR_EACH_ENTRY(dll, &open_dll_list, struct open_dll, entry)
> - {
> -
>    if(hInst == dll->library)
> -
>        dll_list_free(dll);
> - }

That's freeing all the loaded instances, I don't think that's what you
want.

[PATCH v4 1/5] compobj.dll16: Implement CoLoadLibrary16 and CoFreeLibrary16.

[Zebediah Figura]

On 02/17/2017 12:05 PM, Alexandre Julliard wrote:
> Zebediah Figura  writes:
> > +/* --- loaded dll list implementation */
> > +struct open_dll
> > +{
> > - HMODULE16 library;
> > - FARPROC16 DllCanUnloadNow;
> > - struct list entry;
> >
> > +};
> > -
> >
> > +static struct list open_dll_list = LIST_INIT(open_dll_list);
> > -
> >
> > +static HRESULT dll_list_add(LPCSTR library_name, struct open_dll **ret)
> > +{
> > - struct open_dll *dll;
> > - HMODULE16 library;
> > - FARPROC16 DllCanUnloadNow;
> > -
> > - library = LoadLibrary16(library_name);
> > - if (!library)
> > - {
> > -
> >    ERR("couldn't load in-process dll %s\n", debugstr_a(library_name));
> > -
> >    return E_ACCESSDENIED; /* FIXME: or should this be CO_E_DLLNOTFOUND? */
> > - }
> > -
> > - DllCanUnloadNow = GetProcAddress16(library, "DllCanUnloadNow");
> > - /* Note: failing to find DllCanUnloadNow is not a failure */
> > -
> > - dll = HeapAlloc(GetProcessHeap(), 0, sizeof(struct open_dll));
> > - dll->library = library;
> > - dll->DllCanUnloadNow = DllCanUnloadNow;
> > - list_add_tail(&open_dll_list, &dll->entry);
> > - *ret = dll;
> > - TRACE("added new loaded dll %d\n", dll->library);
> > -
> > - return S_OK;
> >
> > +}
>
> I'm not sure we even need the list at all. It would be needed for
> CoFreeAllLibraries16, but it's not clear that this is a useful function.

That's a good point. I'll get rid of these and just implement
CoGetClassObject.