Re: [PATCH] GnuTLS support on Woe32

[Juanma Barranquero <lekktu@gmail.com>]

On Tue, May 3, 2011 at 06:19, Eli Zaretskii <eliz@gnu.org> wrote:

> However, this:
>
>> +#include "lisp.h"
>
> is bad: lisp.h is already included in too many places.  Why did you
> need this?

Because of Lisp_Object. The alternative would be to put Vlibrary_cache
and w32_delayed_load in another module.

>> +/* The argument LIBRARIES is usually the variable
>> +   `dynamic-library-alist', which associates a symbol, identifying
>> +   an external DLL library, to a list of possible filenames.
>> +   The function returns NULL if no library could be loaded for
>> +   the given symbol, or if the library was previously loaded;
>> +   else the handle of the DLL.  */
>> +HMODULE
>> +w32_delayed_load (Lisp_Object libraries, Lisp_Object library_id)
>
> has a couple of problems in the commentary:
>
>  . it should describe the structure of the LIBRARIES alist explicitly,
>   like you'd do in a doc string, not just refer to
>   `dynamic-library-alist'
>
>  . it says nothing about the second argument (and IIUC what it means,
>   the "_id" part of the variable name is misleading)

IIRC, I just moved the code from image.c to here and renamed the
variables. I'll try to fix it.

> Btw, why is it a good idea to return NULL if the library is already
> loaded?  Why not return its handle instead?

Honestly, I asked myself the same question. That was in the image.c
version (written almost seven years ago) and I don't remember why I
thought it was a good idea in the first place. But I was concentrating
in getting GnuTLS loading to work, not on improvements. I'll check.

Thanks for your comments,

    Juanma