Re: [Tarantool-patches] [PATCH] Introduce unified Lua ffi.cdef builder

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the patch!

The patch affects lots of places, so we are likely to have several
review iterations. In this one mostly are major comments. After
they are fixed/rejected, I will dive into every tiny detail.

See 21 comments below.

Also a general comment - it would help to split introduction of ffi
cdef builder, and its usage for each subsystem, into separate commits.

1. Please, add subsystem prefix to the commit title. I think
'lua: ' should be fine here.

On 21/01/2020 14:23, Sergey Voinov wrote:
> Lua ffi is quite fast and simple mechanism allowing to call
> C functions directly, without using Lua C API. It is usually
> faster, sometimes much faster. But ffi requires to define all
> the C methods again in a Lua file in ffi.cdef function. It is
> as bad as any other code duplication - when the original API
> is changed, a one could forget to update Lua files and ffi.cdef
> arguments. It is especially painful, when ffi C definition is really big.
> 
> The goal is to introduce a mechanism to generate ffi C definitions.
> 
> The implementation.
> ===================
> 
> The mechanism to generate ffi C definitions is similar to how
> module.h is generated. There is a script extra/fficdefgen which
> parses specified set of C headers and produces content for ffi.cdef.
> The idea is similar to module.h generation: use comment tags in
> C headers to rip off the content between two tags:
> /* \cond ffi */
> and
> /* \endcond ffi */
> Everything between these tags will be the output of this script.
> 
> The output of extra/fficdefgen is placed into src/lua/load_ffi_defs.lua file.
> This file contains three parts:
> 1. Header - from src/lua/load_ffi_defs.header.lua
> 2. Output from extra/fficdefgen
> 3. Footer - from src/lua/load_ffi_defs.footer.lua
> So we can add some C definitions manually before and after those
> which are automatically generated. This is important since
> some definitions cannot be added automatically, e.g. from system
> headers, submodules and containing identifiers which are Lua keywords
> (see comments in load_ffi_defs.header.lua and load_ffi_defs.footer.lua).
> 
> load_ffi_defs.lua file is produced by rebuild_module_fficdef function in
> cmake/fficdef.cmake CMake script.
> 
> load_ffi_defs.lua is then compiled into src/lua/load_ffi_defs.lua.c source
> (see lua_sources list in CMakeLists.txt).
> 
> load_ffi_defs.lua content is referenced in lua_modules in src/lua/init.c
> and is loaded at startup with all ffi.cdef definitions gathered from
> specified C headers and load_ffi_defs.header.lua/load_ffi_defs.footer.lua.
> 
> So, what we need to include in C headers are the tags.
> But how do we know which C headers to include into load_ffi_defs.lua?
> There is a list (similar to one for module.h) in src/CMakeLists.txt file
> called fficdef_headers. rebuild_module_fficdef function is also
> called from CMakeLists.txt.
> 
> Important notice. For now, only core C headers are included into
> fficdef_headers list. This means that headers from Tarantool
> submodules are not included and some C definitions are thereby
> manually placed into load_ffi_defs.header.lua/load_ffi_defs.footer.lua.
> 
> Currently, this ffi.cdef builder is kind of internal mechanism, but
> can be used by anyone following the scenario:
> 1. Include /* \cond ffi */ and /* \endcond ffi */ tags into needed C header.
> 2. Include needed C header in fficdef_headers list in CMakeLists.txt.
> 
> Closes: #4202
> ---
>  issue: https://github.com/tarantool/tarantool/issues/4202
>  branch: https://github.com/tarantool/tarantool/tree/servoin/gh-4202-ffi-cdef-builder
> diff --git a/cmake/fficdef.cmake b/cmake/fficdef.cmake
> new file mode 100644
> index 000000000..596d0d927
> --- /dev/null
> +++ b/cmake/fficdef.cmake
> @@ -0,0 +1,37 @@
> +# A helper function for unified Lua ffi.cdef builder
> +function(rebuild_module_fficdef)
> +    set (dstfile "${CMAKE_CURRENT_BINARY_DIR}/lua/load_ffi_defs.lua")
> +    set (tmpfile "${dstfile}.new")
> +    set (headers)
> +    # Get absolute path for header files (required of out-of-source build)

2. 'required of' -> 'required for'.

> +    foreach (header ${ARGN})
> +        if (IS_ABSOLUTE ${header})
> +            list(APPEND headers ${header})
> +        else()
> +            list(APPEND headers ${CMAKE_CURRENT_SOURCE_DIR}/${header})
> +        endif()
> +    endforeach()
> +
> +    set (cflags ${CMAKE_C_FLAGS})
> +    separate_arguments(cflags)

3. 'cflags' is never used and can be dropped.

> +    # Pass sysroot settings on OSX
> +    if (NOT "${CMAKE_OSX_SYSROOT}" STREQUAL "")
> +        set (cflags ${cflags} ${CMAKE_C_SYSROOT_FLAG} ${CMAKE_OSX_SYSROOT})
> +    endif()
> +    add_custom_command(OUTPUT ${dstfile}
> +        COMMAND cat ${CMAKE_CURRENT_SOURCE_DIR}/lua/load_ffi_defs.header.lua > ${tmpfile}
> +        COMMAND cat ${headers} | ${CMAKE_SOURCE_DIR}/extra/fficdefgen >> ${tmpfile}
> +        COMMAND cat ${CMAKE_CURRENT_SOURCE_DIR}/lua/load_ffi_defs.footer.lua >> ${tmpfile}
> +        COMMAND ${CMAKE_COMMAND} -E copy_if_different ${tmpfile} ${dstfile}
> +        COMMAND ${CMAKE_COMMAND} -E remove ${errcodefile} ${tmpfile}
> +        DEPENDS ${CMAKE_SOURCE_DIR}/extra/fficdefgen
> +                ${CMAKE_CURRENT_SOURCE_DIR}/lua/load_ffi_defs.header.lua
> +                ${CMAKE_CURRENT_SOURCE_DIR}/lua/load_ffi_defs.footer.lua
> +                ${CMAKE_SOURCE_DIR}/src/box/errcode.h

4. Why depends on errcode.h?

> +                ${headers}
> +        )
> +
> +    add_custom_target(fficdef ALL DEPENDS ${dstfile})
> +    install(FILES ${dstfile} DESTINATION ${MODULE_INCLUDEDIR})
> +endfunction()
> +set_source_files_properties("${CMAKE_CURRENT_BINARY_DIR}/lua/load_ffi_cdef.lua" PROPERTIES GENERATED HEADER_FILE_ONLY)
> diff --git a/extra/fficdefgen b/extra/fficdefgen
> new file mode 100755
> index 000000000..8a63687d3
> --- /dev/null
> +++ b/extra/fficdefgen
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +
> +# This script generates output for unified FFI cdef builder
> +# 1. Include only lines between /** \cond ffi */ and /** \endcond ffi */
> +# 2. Remove API_EXPORT macro
> +# 3. Remove C preprocessor commands (#if defined/#endif)

5. I think we need a more general way to exclude all preprocessor commands.
For example, you missed #elif, #error, #pragma. I propose to use sed to
delete all lines, which start from #.

On the other hand, if an #if or other macros slipped into ffi.cdef, it can
easily break something. For example, consider this code:

	#if SOME_MACRO_CHECK
		int
		func_def(int a, int b);
	#else
		int
		func_def(int a, int b, int c);
	#endif

Your code will define both versions of func_def. IMO, we need to raise
an error, if somebody is trying to add a preprocessor command to ffi cdef.
At 'make' stage. Is it possible to do?

> +# 4. Remove CFORMAT macro
> +# 5. Rename end to epos

6. Why? Inside ffi.cdef() it should be fine to use any keywords,
including 'end'.

> +# 6. Remove MP_PROTO macro

7. For such macros we probably need to create a list of them here, and
delete them in a cycle. To simplify addition of new macros to ignore.

> +
> +sed -n '/^\/\*\* \\cond ffi \*\/$/,/^\/\*\* \\endcond ffi \*\/$/P' | \
> +sed -E 's/([[:space:]]*)API_EXPORT[[:space:]]+/\1/g' | \
> +sed -n -e '/^#if defined/,/^#endif/!p' | \
> +sed -E 's/CFORMAT\(.*\)//g' | \
> +sed -E 's/\*end;/\*epos;/' |
> +sed 's/MP_PROTO//g'
> diff --git a/src/box/index.h b/src/box/index.h
> index 86148023f..0c1288b21 100644
> --- a/src/box/index.h
> +++ b/src/box/index.h
> @@ -47,6 +47,7 @@ struct key_def;
>  struct info_handler;
>  
>  /** \cond public */
> +/** \cond ffi */

8. I would move it above cond public, so it would not
be added to module.h. But up to you, not critical.

>  typedef struct tuple box_tuple_t;
>  typedef struct key_def box_key_def_t;
> @@ -209,6 +210,7 @@ char *
>  box_tuple_extract_key(box_tuple_t *tuple, uint32_t space_id,
>  		      uint32_t index_id, uint32_t *key_size);
>  
> +/** \endcond ffi */
>  /** \endcond public */
>  
>  /**
> diff --git a/src/box/user_def.h b/src/box/user_def.h
> index 486a4ae50..f2c9c6e62 100644
> --- a/src/box/user_def.h
> +++ b/src/box/user_def.h
> @@ -46,6 +46,7 @@ extern "C" {
>  extern const char *CHAP_SHA1_EMPTY_PASSWORD;
>  
>  typedef uint16_t user_access_t;
> +

9. Stray change.

>  /**
>   * Effective session user. A cache of user data
>   * and access stored in session and fiber local storage.
> diff --git a/src/lib/crypto/crypto.h b/src/lib/crypto/crypto.h
> index 027a42735..97f4ed565 100644
> --- a/src/lib/crypto/crypto.h
> +++ b/src/lib/crypto/crypto.h
> @@ -128,6 +134,8 @@ enum crypto_mode {
>  	crypto_mode_MAX,
>  };
>  
> +/** \endcond ffi */
> +
>  extern const char *crypto_mode_strs[];
>  
>  /**
> @@ -151,6 +159,8 @@ enum {
>  	CRYPTO_MAX_BLOCK_SIZE = 16,
>  };
>  
> +/** \cond ffi */
> +
>  /**
>   * OpenSSL API provides generic methods to do both encryption and
>   * decryption depending on one 'int' parameter passed into
> @@ -162,6 +172,8 @@ enum crypto_direction {
>  	CRYPTO_DIR_ENCRYPT = 1,
>  };
>  
> +/** \cond ffi */

10. Double 'cond ffi'. Is it possible to raise an error, when double
cond ffi is met?

> +
>  struct crypto_stream;
>  
>  /**
> diff --git a/src/lua/buffer.lua b/src/lua/buffer.lua
> index ef0118c7c..41ff5bd5c 100644
> --- a/src/lua/buffer.lua
> +++ b/src/lua/buffer.lua
> @@ -3,69 +3,6 @@
>  local ffi = require('ffi')
>  local READAHEAD = 16320
>  
> -ffi.cdef[[
> -struct slab_cache;
> -struct slab_cache *
> -tarantool_lua_slab_cache();
> -extern struct ibuf *tarantool_lua_ibuf;
> -
> -struct ibuf
> -{
> -    struct slab_cache *slabc;
> -    char *buf;
> -    /** Start of input. */
> -    char *rpos;
> -    /** End of useful input */
> -    char *wpos;
> -    /** End of ibuf. */
> -    char *epos;
> -    size_t start_capacity;
> -};
> -
> -void
> -ibuf_create(struct ibuf *ibuf, struct slab_cache *slabc, size_t start_capacity);
> -
> -void
> -ibuf_destroy(struct ibuf *ibuf);
> -
> -void
> -ibuf_reinit(struct ibuf *ibuf);
> -
> -void *
> -ibuf_reserve_slow(struct ibuf *ibuf, size_t size);
> -
> -void *
> -lua_static_aligned_alloc(size_t size, size_t alignment);
> -
> -/**
> - * Register is a buffer to use with FFI functions, which usually
> - * operate with pointers to scalar values like int, char, size_t,
> - * void *. To avoid doing 'ffi.new(<type>[1])' on each such FFI
> - * function invocation, a module can use one of attributes of the
> - * register union.
> - *
> - * Naming policy of the attributes is easy to remember:
> - * 'a' for array type + type name first letters + 'p' for pointer.
> - *
> - * For example:
> - * - int[1] - <a>rray of <i>nt - ai;
> - * - const unsigned char *[1] -
> - *       <a>rray of <c>onst <u>nsigned <c>har <p> pointer - acucp.
> - */
> -union c_register {

11. Please, keep it in this file. We don't need  to remove all ffi.cdef
from all Lua files except one. We need to remove only code duplication.
This union is not defined in C, and is fine to be kept here.

> -    size_t as[1];
> -    void *ap[1];
> -    int ai[1];
> -    char ac[1];
> -    const unsigned char *acucp[1];
> -    unsigned long aul[1];
> -    uint16_t u16;
> -    uint32_t u32;
> -    uint64_t u64;
> -    int64_t i64;
> -};
> -]]
> -
>  local builtin = ffi.C
>  local ibuf_t = ffi.typeof('struct ibuf')
>  > diff --git a/src/lua/load_ffi_defs.footer.lua b/src/lua/load_ffi_defs.footer.lua
> new file mode 100644
> index 000000000..f92065122
> --- /dev/null
> +++ b/src/lua/load_ffi_defs.footer.lua
> @@ -0,0 +1,155 @@
> +
> +]]

12. What is this?

> +
> +-- {{{ Definitions
> +
> +-- POSIX demands to have five fields in struct group:
> +-- char    *pw_name   User's login name.
> +-- uid_t    pw_uid    Numerical user ID.
> +-- gid_t    pw_gid    Numerical group ID.
> +-- char    *pw_dir    Initial working directory.
> +-- char    *pw_shell  Program to use as shell.
> +--
> +-- So we'll extract only them.
> +if ffi.os == 'OSX' or ffi.os == 'BSD' then
> +    ffi.cdef[[
> +        struct passwd {
> +            char    *pw_name;    /* user name */
> +            char    *pw_passwd;  /* encrypted password */
> +            uid_t    pw_uid;     /* user uid */
> +            gid_t    pw_gid;     /* user gid */
> +            time_t   pw_change;  /* password change time */
> +            char    *pw_class;   /* user access class */
> +            char    *pw_gecos;   /* Honeywell login info */
> +            char    *pw_dir;     /* home directory */
> +            char    *pw_shell;   /* default shell */
> +            time_t   pw_expire;  /* account expiration */
> +            int      pw_fields;  /* internal: fields filled in */
> +        };
> +    ]]
> +else
> +    ffi.cdef[[
> +        struct passwd {
> +            char *pw_name;   /* username */
> +            char *pw_passwd; /* user password */
> +            int   pw_uid;    /* user ID */
> +            int   pw_gid;    /* group ID */
> +            char *pw_gecos;  /* user information */
> +            char *pw_dir;    /* home directory */
> +            char *pw_shell;  /* shell program */
> +        };
> +    ]]
> +end
> +
> +-- }}}
> +
> +ffi.cdef[[
> +    /* From src/lib/core/diag.h
> +     * This struct is here because src/lua/error.lua
> +     * uses underscored member names
> +     * (i.e. we cannot use 'type' member since it's a keyword)

13. Nope. 'Type' can be used right away. 'End' can't be used
that easy (surprisingly), but can be if inside [''].

	ffi.cdef[[
	    struct example {
	        int end;
	        int type;
	    };
	]]

	e = ffi.new('struct example')

	tarantool> e.type
	---
	- 0
	...

	tarantool> e['end']
	---
	- 0
	...

Please, let struct error be generated from ffi tags. And
remove '_' prefixes in error.lua file, where the struct error
attributes are used.

> +     */
> +    struct error {
> +        error_f _destroy;
> +        error_f _raise;
> +        error_f _log;
> +        const struct type_info *_type;
> +        int _refs;
> +        int _saved_errno;
> +        /** Line number. */
> +        unsigned _line;
> +        /* Source file name. */
> +        char _file[DIAG_FILENAME_MAX];
> +        /* Error description. */
> +        char _errmsg[DIAG_ERRMSG_MAX];
> +    };
> +
> +    /* From src/box/user_def.h
> +     * This enum is here because Lua C parser
> +     * incorrecly assigns PRIV_ALL = -1

14. PRIV_ALL is defined as '~0' in user_def.h. It should work.

	ffi.cdef[[
	    enum my_enum {
	        VALUE = 1,
	        VALUE2 = ~0,
	    };
	]]

	e = ffi.C.my_enum.VALUE

	tarantool> ffi.C.VALUE
	---
	- 1
	...

	tarantool> ffi.C.VALUE2
	---
	- -1
	...

> +     * - this is signed type, and unsigned is required
> +     */
> +    enum priv_type {
> +        /* SELECT */
> +        PRIV_R = 1,
> +        /* INSERT, UPDATE, UPSERT, DELETE, REPLACE */
> +        PRIV_W = 2,
> +        /* CALL */
> +        PRIV_X = 4,
> +        /* SESSION */
> +        PRIV_S = 8,
> +        /* USAGE */
> +        PRIV_U = 16,
> +        /* CREATE */
> +        PRIV_C = 32,
> +        /* DROP */
> +        PRIV_D = 64,
> +        /* ALTER */
> +        PRIV_A = 128,
> +        /* REFERENCE - required by ANSI - not implemented */
> +        PRIV_REFERENCE = 256,
> +        /* TRIGGER - required by ANSI - not implemented */
> +        PRIV_TRIGGER = 512,
> +        /* INSERT - required by ANSI - not implemented */
> +        PRIV_INSERT = 1024,
> +        /* UPDATE - required by ANSI - not implemented */
> +        PRIV_UPDATE = 2048,
> +        /* DELETE - required by ANSI - not implemented */
> +        PRIV_DELETE = 4096,
> +        /* This is never granted, but used internally. */
> +        PRIV_GRANT = 8192,
> +        /* Never granted, but used internally. */
> +        PRIV_REVOKE = 16384,
> +        /* all bits */
> +        PRIV_ALL  = 4294967295
> +    };
> +]]
> +
> +ffi.cdef[[
> +    /* From src/lib/small/ibuf.h (submodule) */

15. I think we need to be able to parse submodule files
too. Don't see any reason why would it be impossible. Yes,
it would require pushing a change into submodule repository,
but it is better than code duplication.

> +
> +    struct ibuf
> +    {
> +        struct slab_cache *slabc;
> +        char *buf;
> +        /** Start of input. */
> +        char *rpos;
> +        /** End of useful input */
> +        char *wpos;
> +        /** End of ibuf. */
> +        char *epos;
> +        size_t start_capacity;
> +    };
> +
> +    void
> +    ibuf_create(struct ibuf *ibuf, struct slab_cache *slabc, size_t start_capacity);
> +
> +    void
> +    ibuf_destroy(struct ibuf *ibuf);
> +
> +    void
> +    ibuf_reinit(struct ibuf *ibuf);
> +
> +    void *
> +    ibuf_reserve_slow(struct ibuf *ibuf, size_t size);
> +]]
> +
> +ffi.cdef[[
> +    /* From src/lib/msgpuck/msgpuck.h (submodule) */
> +
> +    uint32_t
> +    mp_decode_extl(const char **data, int8_t *type);
> +
> +    char *
> +    mp_encode_float(char *data, float num);
> +
> +    char *
> +    mp_encode_double(char *data, double num);
> +
> +    float
> +    mp_decode_float(const char **data);
> +
> +    double
> +    mp_decode_double(const char **data);
> +]]
> +
> diff --git a/src/lua/load_ffi_defs.header.lua b/src/lua/load_ffi_defs.header.lua
> new file mode 100755
> index 000000000..da88d5b25
> --- /dev/null
> +++ b/src/lua/load_ffi_defs.header.lua
> @@ -0,0 +1,187 @@
> +---
> +--- Unified Lua ffi.cdef builder header file.
> +--- The definitions below are either from system headers
> +--- or from dependent packages or tarantol repo files
> +--- which cannot be automatically preprocessed.
> +---
> +
> +ffi = require('ffi')
> +
> +ffi.cdef[[
> +/* from libc */
> +
> +char *strerror(int errnum);
> +int snprintf(char *str, size_t size, const char *format, ...);
> +const char *memmem(const char *haystack, size_t haystack_len,
> +                   const char *needle, size_t needle_len);
> +int memcmp(const char *mem1, const char *mem2, size_t num);
> +int isspace(int c);
> +typedef int32_t pid_t;
> +pid_t getpid(void);

16. I have an idea about libc definitions. Currently these declarations
can't be checked if they are correct, since they are added at runtime.
But we could add them to a header file and include into a C file. Then

1) a compiler would check that these definitions are correct;
2) we could surround them with cond ffi and don't add into the Lua file
  manually.

Although a compiler may decide to drop our definitions, if it will see
that they are unused. I know a workaround for this though.

This may be done as a separate patch or even ticket.

> +
> +// GID_T, UID_T and TIME_T are, essentially, `integer types`.
> +// http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/types.h.html

17. We never use // comments. Please, use /* syntax. '//' may be present in
some old code, but in new code /* is right.

> +typedef int uid_t;
> +typedef int gid_t;
> +typedef long time_t;

18. To be totally sure that this is correct, we could move these definitions
to the header file, mentioned by me above, and add static assertions.

> +
> +// POSIX demands to have three fields in struct group:
> +// http://pubs.opengroup.org/onlinepubs/009695399/basedefs/grp.h.html
> +// char   *gr_name The name of the group.
> +// gid_t   gr_gid  Numerical group ID.
> +// char  **gr_mem  Pointer to a null-terminated array of character pointers to
> +//                 member names.
> +//
> +// So we'll extract only them.
> +struct group {
> +    char    *gr_name;    /* group name */
> +    char    *gr_passwd;  /* group password */
> +    gid_t    gr_gid;     /* group id */
> +    char   **gr_mem;     /* group members */
> +};
> +
> +uid_t          getuid();
> +struct passwd *getpwuid(uid_t uid);
> +struct passwd *getpwnam(const char *login);
> +void           endpwent();
> +struct passwd *getpwent();
> +void           setpwent();
> +gid_t          getgid();
> +struct group  *getgrgid(gid_t gid);
> +struct group  *getgrnam(const char *group);
> +struct group  *getgrent();
> +void           endgrent();
> +void           setgrent();
> +
> +int umask(int mask);
> +char *dirname(char *path);
> +int chdir(const char *path);
> +
> +struct gc_socket {
> +    const int fd;
> +};

19. Such heavily module-local definitions should be kept inside their
modules. They were defined in Lua only, in a single place, so their
move to this Lua 'header' does not reduce duplication, but breaks
locality of definitions.

> +
> +typedef uint32_t socklen_t;
> +typedef ptrdiff_t ssize_t;
> +struct sockaddr;
> +
> +int connect(int sockfd, const struct sockaddr *addr,
> +            socklen_t addrlen);
> +int bind(int sockfd, const struct sockaddr *addr,
> +         socklen_t addrlen);
> +ssize_t write(int fd, const char *octets, size_t len);
> +ssize_t read(int fd, void *buf, size_t count);
> +int listen(int fd, int backlog);
> +int socket(int domain, int type, int protocol);
> +int shutdown(int s, int how);
> +ssize_t send(int sockfd, const void *buf, size_t len, int flags);
> +ssize_t recv(int s, void *buf, size_t len, int flags);
> +int accept(int s, void *addr, void *addrlen);
> +ssize_t sendto(int sockfd, const void *buf, size_t len, int flags,
> +               const struct sockaddr *dest_addr, socklen_t addrlen);
> +int setsockopt(int s, int level, int iname, const void *opt, size_t optlen);
> +int getsockopt(int s, int level, int iname, void *ptr, size_t *optlen);
> +
> +typedef struct { int active; int timeout; } linger_t;
> +
> +struct protoent {
> +    char  *p_name;       /* official protocol name */
> +    char **p_aliases;    /* alias list */
> +    int    p_proto;      /* protocol number */
> +};
> +struct protoent *getprotobyname(const char *name);
> +
> +extern char **environ;
> +int setenv(const char *name, const char *value, int overwrite);
> +int unsetenv(const char *name);
> +
> +/* from third_party/base64.h */
> +int base64_bufsize(int binsize, int options);
> +int base64_decode(const char *in_base64, int in_len, char *out_bin, int out_len);
> +int base64_encode(const char *in_bin, int in_len, char *out_base64, int out_len, int options);
> +
> +/* from third_party/PMurHash.h */
> +void PMurHash32_Process(uint32_t *ph1, uint32_t *pcarry, const void *key, int len);
> +uint32_t PMurHash32_Result(uint32_t h1, uint32_t carry, uint32_t total_length);
> +uint32_t PMurHash32(uint32_t seed, const void *key, int len);
> +
> +/* from <iconv.h> */
> +typedef struct iconv *iconv_t;
> +
> +/* from src/lua/tnt_iconv.c */
> +iconv_t tnt_iconv_open(const char *tocode, const char *fromcode);
> +void    tnt_iconv_close(iconv_t cd);
> +size_t  tnt_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft,
> +                  char **outbuf, size_t *outbytesleft);
> +
> +/* from openssl/engine.h */
> +typedef void ENGINE;
> +
> +/* from openssl/err.h */
> +unsigned long ERR_get_error(void);
> +char *ERR_error_string(unsigned long e, char *buf);
> +
> +/* from openssl/evp.h */
> +typedef struct {} EVP_MD_CTX;
> +typedef struct {} EVP_MD;
> +int EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *impl);
> +int EVP_DigestUpdate(EVP_MD_CTX *ctx, const void *d, size_t cnt);
> +int EVP_DigestFinal_ex(EVP_MD_CTX *ctx, unsigned char *md, unsigned int *s);
> +const EVP_MD *EVP_get_digestbyname(const char *name);
> +
> +/* from openssl/hmac.h */
> +typedef struct {} HMAC_CTX;
> +int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len,
> +                 const EVP_MD *md, ENGINE *impl);
> +int HMAC_Update(HMAC_CTX *ctx, const unsigned char *data, size_t len);
> +int HMAC_Final(HMAC_CTX *ctx, unsigned char *md, unsigned int *len);
> +
> +/* from src/lib/crypto/crypto.c */
> +EVP_MD_CTX *crypto_EVP_MD_CTX_new(void);
> +void crypto_EVP_MD_CTX_free(EVP_MD_CTX *ctx);
> +HMAC_CTX *crypto_HMAC_CTX_new(void);
> +void crypto_HMAC_CTX_free(HMAC_CTX *ctx);
> +
> +/* from src/lib/uuid/tt_uuid.h (inline functions) */
> +struct tt_uuid;
> +int tt_uuid_from_string(const char *in, struct tt_uuid *uu);
> +void tt_uuid_to_string(const struct tt_uuid *uu, char *out);
> +void tt_uuid_bswap(struct tt_uuid *uu);
> +bool tt_uuid_is_nil(const struct tt_uuid *uu);
> +bool tt_uuid_is_equal(const struct tt_uuid *lhs, const struct tt_uuid *rhs);
> +
> +/* from src/lua/init.h (declaration of tarantool_lua_slab_cache) */
> +struct slab_cache;

20. I propose to add such things to their C headers inside ffi
tags.

> diff --git a/test/app/uuid.result b/test/app/uuid.result
> index 0713614c6..bb896e2b1 100644
> --- a/test/app/uuid.result
> +++ b/test/app/uuid.result
> @@ -106,7 +106,7 @@ uu.node[5]
>  -- invalid values
>  uuid.fromstr(nil)
>  ---
> -- error: 'builtin/uuid.lua:47: fromstr(str)'
> +- error: 'builtin/uuid.lua:20: fromstr(str)'

21. While we are here, lets add a test_run filter to mask out line
numbers.

>  ...
>  uuid.fromstr('')
>  ---
>