* [Tarantool-patches] [PATCH] Ensure all curl symbols are exported
@ 2020-08-06 13:19 Yaroslav Dynnikov
2020-08-06 15:06 ` Timur Safin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Yaroslav Dynnikov @ 2020-08-06 13:19 UTC (permalink / raw)
To: tarantool-patches
Cc: Yaroslav Dynnikov, Vladislav Shpilevoy, Alexander Turenko
In the recent update of libcurl (2.5.0-278-g807c7fa58) its layout has
changed: private function `Curl_version_init()` which used to fill-in
info structure was eliminated. As a result, no symbols for
`libcurl_la-version.o` remained used, so it wasn't included in tarantool
binary. And `curl_version` and `curl_version_info` symbols went missing.
According to libcurl naming conventions all exported symbols are named
as `curl_*`. This patch lists them all explicitly in `exprots.h` and
adds the test.
Close #5223
Issue: https://github.com/tarantool/tarantool/issues/5223
Branch: https://github.com/tarantool/tarantool/tree/rosik/gh-5223-missing-curl-symbols
---
src/exports.h | 81 ++++++++++
test/box-tap/gh-5223-curl-exports.test.lua | 177 +++++++++++++++++++++
2 files changed, 258 insertions(+)
create mode 100755 test/box-tap/gh-5223-curl-exports.test.lua
diff --git a/src/exports.h b/src/exports.h
index 7cf283e5b..7f0601f4f 100644
--- a/src/exports.h
+++ b/src/exports.h
@@ -113,6 +113,87 @@ EXPORT(csv_feed)
EXPORT(csv_iterator_create)
EXPORT(csv_next)
EXPORT(csv_setopt)
+EXPORT(curl_easy_cleanup)
+EXPORT(curl_easy_duphandle)
+EXPORT(curl_easy_escape)
+EXPORT(curl_easy_getinfo)
+EXPORT(curl_easy_init)
+EXPORT(curl_easy_pause)
+EXPORT(curl_easy_perform)
+EXPORT(curl_easy_recv)
+EXPORT(curl_easy_reset)
+EXPORT(curl_easy_send)
+EXPORT(curl_easy_setopt)
+EXPORT(curl_easy_strerror)
+EXPORT(curl_easy_unescape)
+EXPORT(curl_easy_upkeep)
+EXPORT(curl_escape)
+EXPORT(curl_formadd)
+EXPORT(curl_formfree)
+EXPORT(curl_formget)
+EXPORT(curl_free)
+EXPORT(curl_getdate)
+EXPORT(curl_getenv)
+EXPORT(curl_global_cleanup)
+EXPORT(curl_global_init)
+EXPORT(curl_global_init_mem)
+EXPORT(curl_global_sslset)
+EXPORT(curl_maprintf)
+EXPORT(curl_mfprintf)
+EXPORT(curl_mime_addpart)
+EXPORT(curl_mime_data)
+EXPORT(curl_mime_data_cb)
+EXPORT(curl_mime_encoder)
+EXPORT(curl_mime_filedata)
+EXPORT(curl_mime_filename)
+EXPORT(curl_mime_free)
+EXPORT(curl_mime_headers)
+EXPORT(curl_mime_init)
+EXPORT(curl_mime_name)
+EXPORT(curl_mime_subparts)
+EXPORT(curl_mime_type)
+EXPORT(curl_mprintf)
+EXPORT(curl_msnprintf)
+EXPORT(curl_msprintf)
+EXPORT(curl_multi_add_handle)
+EXPORT(curl_multi_assign)
+EXPORT(curl_multi_cleanup)
+EXPORT(curl_multi_fdset)
+EXPORT(curl_multi_info_read)
+EXPORT(curl_multi_init)
+EXPORT(curl_multi_perform)
+EXPORT(curl_multi_poll)
+EXPORT(curl_multi_remove_handle)
+EXPORT(curl_multi_setopt)
+EXPORT(curl_multi_socket)
+EXPORT(curl_multi_socket_action)
+EXPORT(curl_multi_socket_all)
+EXPORT(curl_multi_strerror)
+EXPORT(curl_multi_timeout)
+EXPORT(curl_multi_wait)
+EXPORT(curl_mvaprintf)
+EXPORT(curl_mvfprintf)
+EXPORT(curl_mvprintf)
+EXPORT(curl_mvsnprintf)
+EXPORT(curl_mvsprintf)
+EXPORT(curl_pushheader_byname)
+EXPORT(curl_pushheader_bynum)
+EXPORT(curl_share_cleanup)
+EXPORT(curl_share_init)
+EXPORT(curl_share_setopt)
+EXPORT(curl_share_strerror)
+EXPORT(curl_slist_append)
+EXPORT(curl_slist_free_all)
+EXPORT(curl_strequal)
+EXPORT(curl_strnequal)
+EXPORT(curl_unescape)
+EXPORT(curl_url)
+EXPORT(curl_url_cleanup)
+EXPORT(curl_url_dup)
+EXPORT(curl_url_get)
+EXPORT(curl_url_set)
+EXPORT(curl_version)
+EXPORT(curl_version_info)
EXPORT(decimal_unpack)
EXPORT(error_ref)
EXPORT(error_set_prev)
diff --git a/test/box-tap/gh-5223-curl-exports.test.lua b/test/box-tap/gh-5223-curl-exports.test.lua
new file mode 100755
index 000000000..300d60b07
--- /dev/null
+++ b/test/box-tap/gh-5223-curl-exports.test.lua
@@ -0,0 +1,177 @@
+#!/usr/bin/env tarantool
+
+local tap = require('tap')
+local ffi = require('ffi')
+ffi.cdef([[
+ void *dlsym(void *handle, const char *symbol);
+ struct curl_version_info_data {
+ int age; /* see description below */
+ const char *version; /* human readable string */
+ unsigned int version_num; /* numeric representation */
+ const char *host; /* human readable string */
+ int features; /* bitmask, see below */
+ char *ssl_version; /* human readable string */
+ long ssl_version_num; /* not used, always zero */
+ const char *libz_version; /* human readable string */
+ const char * const *protocols; /* protocols */
+
+ /* when 'age' is CURLVERSION_SECOND or higher, the members below exist */
+ const char *ares; /* human readable string */
+ int ares_num; /* number */
+
+ /* when 'age' is CURLVERSION_THIRD or higher, the members below exist */
+ const char *libidn; /* human readable string */
+
+ /* when 'age' is CURLVERSION_FOURTH or higher (>= 7.16.1), the members
+ below exist */
+ int iconv_ver_num; /* '_libiconv_version' if iconv support enabled */
+
+ const char *libssh_version; /* human readable string */
+
+ /* when 'age' is CURLVERSION_FIFTH or higher (>= 7.57.0), the members
+ below exist */
+ unsigned int brotli_ver_num; /* Numeric Brotli version
+ (MAJOR << 24) | (MINOR << 12) | PATCH */
+ const char *brotli_version; /* human readable string. */
+
+ /* when 'age' is CURLVERSION_SIXTH or higher (>= 7.66.0), the members
+ below exist */
+ unsigned int nghttp2_ver_num; /* Numeric nghttp2 version
+ (MAJOR << 16) | (MINOR << 8) | PATCH */
+ const char *nghttp2_version; /* human readable string. */
+
+ const char *quic_version; /* human readable quic (+ HTTP/3) library +
+ version or NULL */
+
+ /* when 'age' is CURLVERSION_SEVENTH or higher (>= 7.70.0), the members
+ below exist */
+ const char *cainfo; /* the built-in default CURLOPT_CAINFO, might
+ be NULL */
+ const char *capath; /* the built-in default CURLOPT_CAPATH, might
+ be NULL */
+ };
+
+ struct curl_version_info_data *curl_version_info(int age);
+]])
+
+local info = ffi.C.curl_version_info(7)
+local test = tap.test('curl-features')
+test:plan(3)
+
+if test:ok(info.ssl_version ~= nil, 'Curl built with SSL support') then
+ test:diag('ssl_version: ' .. ffi.string(info.ssl_version))
+end
+if test:ok(info.libz_version ~= nil, 'Curl built with LIBZ') then
+ test:diag('libz_version: ' .. ffi.string(info.libz_version))
+end
+
+local RTLD_DEFAULT
+-- See `man 3 dlsym`:
+-- RTLD_DEFAULT
+-- Find the first occurrence of the desired symbol using the default
+-- shared object search order. The search will include global symbols
+-- in the executable and its dependencies, as well as symbols in shared
+-- objects that were dynamically loaded with the RTLD_GLOBAL flag.
+if jit.os == "OSX" then
+ RTLD_DEFAULT = ffi.cast("void *", -2LL)
+else
+ RTLD_DEFAULT = ffi.cast("void *", 0LL)
+end
+
+-- The following list was obtained by parsing libcurl.a static library:
+-- nm libcurl.a | grep -oP 'T \K(curl_.+)$' | sort
+local curl_symbols = {
+ 'curl_easy_cleanup',
+ 'curl_easy_duphandle',
+ 'curl_easy_escape',
+ 'curl_easy_getinfo',
+ 'curl_easy_init',
+ 'curl_easy_pause',
+ 'curl_easy_perform',
+ 'curl_easy_recv',
+ 'curl_easy_reset',
+ 'curl_easy_send',
+ 'curl_easy_setopt',
+ 'curl_easy_strerror',
+ 'curl_easy_unescape',
+ 'curl_easy_upkeep',
+ 'curl_escape',
+ 'curl_formadd',
+ 'curl_formfree',
+ 'curl_formget',
+ 'curl_free',
+ 'curl_getdate',
+ 'curl_getenv',
+ 'curl_global_cleanup',
+ 'curl_global_init',
+ 'curl_global_init_mem',
+ 'curl_global_sslset',
+ 'curl_maprintf',
+ 'curl_mfprintf',
+ 'curl_mime_addpart',
+ 'curl_mime_data',
+ 'curl_mime_data_cb',
+ 'curl_mime_encoder',
+ 'curl_mime_filedata',
+ 'curl_mime_filename',
+ 'curl_mime_free',
+ 'curl_mime_headers',
+ 'curl_mime_init',
+ 'curl_mime_name',
+ 'curl_mime_subparts',
+ 'curl_mime_type',
+ 'curl_mprintf',
+ 'curl_msnprintf',
+ 'curl_msprintf',
+ 'curl_multi_add_handle',
+ 'curl_multi_assign',
+ 'curl_multi_cleanup',
+ 'curl_multi_fdset',
+ 'curl_multi_info_read',
+ 'curl_multi_init',
+ 'curl_multi_perform',
+ 'curl_multi_poll',
+ 'curl_multi_remove_handle',
+ 'curl_multi_setopt',
+ 'curl_multi_socket',
+ 'curl_multi_socket_action',
+ 'curl_multi_socket_all',
+ 'curl_multi_strerror',
+ 'curl_multi_timeout',
+ 'curl_multi_wait',
+ 'curl_mvaprintf',
+ 'curl_mvfprintf',
+ 'curl_mvprintf',
+ 'curl_mvsnprintf',
+ 'curl_mvsprintf',
+ 'curl_pushheader_byname',
+ 'curl_pushheader_bynum',
+ 'curl_share_cleanup',
+ 'curl_share_init',
+ 'curl_share_setopt',
+ 'curl_share_strerror',
+ 'curl_slist_append',
+ 'curl_slist_free_all',
+ 'curl_strequal',
+ 'curl_strnequal',
+ 'curl_unescape',
+ 'curl_url',
+ 'curl_url_cleanup',
+ 'curl_url_dup',
+ 'curl_url_get',
+ 'curl_url_set',
+ 'curl_version',
+ 'curl_version_info',
+}
+
+test:test('curl_symbols', function(t)
+ t:plan(#curl_symbols)
+ for _, sym in ipairs(curl_symbols) do
+ t:ok(
+ ffi.C.dlsym(RTLD_DEFAULT, sym) ~= nil,
+ ('Symbol %q found'):format(sym)
+ )
+ end
+end)
+
+os.exit(test:check() and 0 or 1)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH] Ensure all curl symbols are exported
2020-08-06 13:19 [Tarantool-patches] [PATCH] Ensure all curl symbols are exported Yaroslav Dynnikov
@ 2020-08-06 15:06 ` Timur Safin
2020-08-10 22:49 ` Vladislav Shpilevoy
2020-08-13 11:48 ` Kirill Yukhin
2 siblings, 0 replies; 6+ messages in thread
From: Timur Safin @ 2020-08-06 15:06 UTC (permalink / raw)
To: 'Yaroslav Dynnikov', tarantool-patches
Cc: 'Vladislav Shpilevoy', 'Alexander Turenko'
Much respect for the outstanding symbol loader test!
LGTM
Timur
: From: Tarantool-patches <tarantool-patches-bounces@dev.tarantool.org> On
: Behalf Of Yaroslav Dynnikov
: Subject: [Tarantool-patches] [PATCH] Ensure all curl symbols are exported
:
: In the recent update of libcurl (2.5.0-278-g807c7fa58) its layout has
: changed: private function `Curl_version_init()` which used to fill-in
: info structure was eliminated. As a result, no symbols for
: `libcurl_la-version.o` remained used, so it wasn't included in tarantool
: binary. And `curl_version` and `curl_version_info` symbols went missing.
:
: According to libcurl naming conventions all exported symbols are named
: as `curl_*`. This patch lists them all explicitly in `exprots.h` and
: adds the test.
:
: Close #5223
:
: Issue: https://github.com/tarantool/tarantool/issues/5223
: Branch: https://github.com/tarantool/tarantool/tree/rosik/gh-5223-missing-
: curl-symbols
:
: ---
: src/exports.h | 81 ++++++++++
: test/box-tap/gh-5223-curl-exports.test.lua | 177 +++++++++++++++++++++
: 2 files changed, 258 insertions(+)
: create mode 100755 test/box-tap/gh-5223-curl-exports.test.lua
:
: diff --git a/src/exports.h b/src/exports.h
: index 7cf283e5b..7f0601f4f 100644
: --- a/src/exports.h
: +++ b/src/exports.h
: @@ -113,6 +113,87 @@ EXPORT(csv_feed)
: EXPORT(csv_iterator_create)
: EXPORT(csv_next)
: EXPORT(csv_setopt)
: +EXPORT(curl_easy_cleanup)
: +EXPORT(curl_easy_duphandle)
: +EXPORT(curl_easy_escape)
: +EXPORT(curl_easy_getinfo)
: +EXPORT(curl_easy_init)
: +EXPORT(curl_easy_pause)
: +EXPORT(curl_easy_perform)
: +EXPORT(curl_easy_recv)
: +EXPORT(curl_easy_reset)
: +EXPORT(curl_easy_send)
: +EXPORT(curl_easy_setopt)
: +EXPORT(curl_easy_strerror)
: +EXPORT(curl_easy_unescape)
: +EXPORT(curl_easy_upkeep)
: +EXPORT(curl_escape)
: +EXPORT(curl_formadd)
: +EXPORT(curl_formfree)
: +EXPORT(curl_formget)
: +EXPORT(curl_free)
: +EXPORT(curl_getdate)
: +EXPORT(curl_getenv)
: +EXPORT(curl_global_cleanup)
: +EXPORT(curl_global_init)
: +EXPORT(curl_global_init_mem)
: +EXPORT(curl_global_sslset)
: +EXPORT(curl_maprintf)
: +EXPORT(curl_mfprintf)
: +EXPORT(curl_mime_addpart)
: +EXPORT(curl_mime_data)
: +EXPORT(curl_mime_data_cb)
: +EXPORT(curl_mime_encoder)
: +EXPORT(curl_mime_filedata)
: +EXPORT(curl_mime_filename)
: +EXPORT(curl_mime_free)
: +EXPORT(curl_mime_headers)
: +EXPORT(curl_mime_init)
: +EXPORT(curl_mime_name)
: +EXPORT(curl_mime_subparts)
: +EXPORT(curl_mime_type)
: +EXPORT(curl_mprintf)
: +EXPORT(curl_msnprintf)
: +EXPORT(curl_msprintf)
: +EXPORT(curl_multi_add_handle)
: +EXPORT(curl_multi_assign)
: +EXPORT(curl_multi_cleanup)
: +EXPORT(curl_multi_fdset)
: +EXPORT(curl_multi_info_read)
: +EXPORT(curl_multi_init)
: +EXPORT(curl_multi_perform)
: +EXPORT(curl_multi_poll)
: +EXPORT(curl_multi_remove_handle)
: +EXPORT(curl_multi_setopt)
: +EXPORT(curl_multi_socket)
: +EXPORT(curl_multi_socket_action)
: +EXPORT(curl_multi_socket_all)
: +EXPORT(curl_multi_strerror)
: +EXPORT(curl_multi_timeout)
: +EXPORT(curl_multi_wait)
: +EXPORT(curl_mvaprintf)
: +EXPORT(curl_mvfprintf)
: +EXPORT(curl_mvprintf)
: +EXPORT(curl_mvsnprintf)
: +EXPORT(curl_mvsprintf)
: +EXPORT(curl_pushheader_byname)
: +EXPORT(curl_pushheader_bynum)
: +EXPORT(curl_share_cleanup)
: +EXPORT(curl_share_init)
: +EXPORT(curl_share_setopt)
: +EXPORT(curl_share_strerror)
: +EXPORT(curl_slist_append)
: +EXPORT(curl_slist_free_all)
: +EXPORT(curl_strequal)
: +EXPORT(curl_strnequal)
: +EXPORT(curl_unescape)
: +EXPORT(curl_url)
: +EXPORT(curl_url_cleanup)
: +EXPORT(curl_url_dup)
: +EXPORT(curl_url_get)
: +EXPORT(curl_url_set)
: +EXPORT(curl_version)
: +EXPORT(curl_version_info)
: EXPORT(decimal_unpack)
: EXPORT(error_ref)
: EXPORT(error_set_prev)
: diff --git a/test/box-tap/gh-5223-curl-exports.test.lua b/test/box-tap/gh-
: 5223-curl-exports.test.lua
: new file mode 100755
: index 000000000..300d60b07
: --- /dev/null
: +++ b/test/box-tap/gh-5223-curl-exports.test.lua
: @@ -0,0 +1,177 @@
: +#!/usr/bin/env tarantool
: +
: +local tap = require('tap')
: +local ffi = require('ffi')
: +ffi.cdef([[
: + void *dlsym(void *handle, const char *symbol);
: + struct curl_version_info_data {
: + int age; /* see description below */
: + const char *version; /* human readable string */
: + unsigned int version_num; /* numeric representation */
: + const char *host; /* human readable string */
: + int features; /* bitmask, see below */
: + char *ssl_version; /* human readable string */
: + long ssl_version_num; /* not used, always zero */
: + const char *libz_version; /* human readable string */
: + const char * const *protocols; /* protocols */
: +
: + /* when 'age' is CURLVERSION_SECOND or higher, the members below
: exist */
: + const char *ares; /* human readable string */
: + int ares_num; /* number */
: +
: + /* when 'age' is CURLVERSION_THIRD or higher, the members below
: exist */
: + const char *libidn; /* human readable string */
: +
: + /* when 'age' is CURLVERSION_FOURTH or higher (>= 7.16.1), the
: members
: + below exist */
: + int iconv_ver_num; /* '_libiconv_version' if iconv support
: enabled */
: +
: + const char *libssh_version; /* human readable string */
: +
: + /* when 'age' is CURLVERSION_FIFTH or higher (>= 7.57.0), the
: members
: + below exist */
: + unsigned int brotli_ver_num; /* Numeric Brotli version
: + (MAJOR << 24) | (MINOR << 12) |
: PATCH */
: + const char *brotli_version; /* human readable string. */
: +
: + /* when 'age' is CURLVERSION_SIXTH or higher (>= 7.66.0), the
: members
: + below exist */
: + unsigned int nghttp2_ver_num; /* Numeric nghttp2 version
: + (MAJOR << 16) | (MINOR << 8) |
: PATCH */
: + const char *nghttp2_version; /* human readable string. */
: +
: + const char *quic_version; /* human readable quic (+ HTTP/3)
: library +
: + version or NULL */
: +
: + /* when 'age' is CURLVERSION_SEVENTH or higher (>= 7.70.0), the
: members
: + below exist */
: + const char *cainfo; /* the built-in default
: CURLOPT_CAINFO, might
: + be NULL */
: + const char *capath; /* the built-in default
: CURLOPT_CAPATH, might
: + be NULL */
: + };
: +
: + struct curl_version_info_data *curl_version_info(int age);
: +]])
: +
: +local info = ffi.C.curl_version_info(7)
: +local test = tap.test('curl-features')
: +test:plan(3)
: +
: +if test:ok(info.ssl_version ~= nil, 'Curl built with SSL support') then
: + test:diag('ssl_version: ' .. ffi.string(info.ssl_version))
: +end
: +if test:ok(info.libz_version ~= nil, 'Curl built with LIBZ') then
: + test:diag('libz_version: ' .. ffi.string(info.libz_version))
: +end
: +
: +local RTLD_DEFAULT
: +-- See `man 3 dlsym`:
: +-- RTLD_DEFAULT
: +-- Find the first occurrence of the desired symbol using the default
: +-- shared object search order. The search will include global symbols
: +-- in the executable and its dependencies, as well as symbols in shared
: +-- objects that were dynamically loaded with the RTLD_GLOBAL flag.
: +if jit.os == "OSX" then
: + RTLD_DEFAULT = ffi.cast("void *", -2LL)
: +else
: + RTLD_DEFAULT = ffi.cast("void *", 0LL)
: +end
: +
: +-- The following list was obtained by parsing libcurl.a static library:
: +-- nm libcurl.a | grep -oP 'T \K(curl_.+)$' | sort
: +local curl_symbols = {
: + 'curl_easy_cleanup',
: + 'curl_easy_duphandle',
: + 'curl_easy_escape',
: + 'curl_easy_getinfo',
: + 'curl_easy_init',
: + 'curl_easy_pause',
: + 'curl_easy_perform',
: + 'curl_easy_recv',
: + 'curl_easy_reset',
: + 'curl_easy_send',
: + 'curl_easy_setopt',
: + 'curl_easy_strerror',
: + 'curl_easy_unescape',
: + 'curl_easy_upkeep',
: + 'curl_escape',
: + 'curl_formadd',
: + 'curl_formfree',
: + 'curl_formget',
: + 'curl_free',
: + 'curl_getdate',
: + 'curl_getenv',
: + 'curl_global_cleanup',
: + 'curl_global_init',
: + 'curl_global_init_mem',
: + 'curl_global_sslset',
: + 'curl_maprintf',
: + 'curl_mfprintf',
: + 'curl_mime_addpart',
: + 'curl_mime_data',
: + 'curl_mime_data_cb',
: + 'curl_mime_encoder',
: + 'curl_mime_filedata',
: + 'curl_mime_filename',
: + 'curl_mime_free',
: + 'curl_mime_headers',
: + 'curl_mime_init',
: + 'curl_mime_name',
: + 'curl_mime_subparts',
: + 'curl_mime_type',
: + 'curl_mprintf',
: + 'curl_msnprintf',
: + 'curl_msprintf',
: + 'curl_multi_add_handle',
: + 'curl_multi_assign',
: + 'curl_multi_cleanup',
: + 'curl_multi_fdset',
: + 'curl_multi_info_read',
: + 'curl_multi_init',
: + 'curl_multi_perform',
: + 'curl_multi_poll',
: + 'curl_multi_remove_handle',
: + 'curl_multi_setopt',
: + 'curl_multi_socket',
: + 'curl_multi_socket_action',
: + 'curl_multi_socket_all',
: + 'curl_multi_strerror',
: + 'curl_multi_timeout',
: + 'curl_multi_wait',
: + 'curl_mvaprintf',
: + 'curl_mvfprintf',
: + 'curl_mvprintf',
: + 'curl_mvsnprintf',
: + 'curl_mvsprintf',
: + 'curl_pushheader_byname',
: + 'curl_pushheader_bynum',
: + 'curl_share_cleanup',
: + 'curl_share_init',
: + 'curl_share_setopt',
: + 'curl_share_strerror',
: + 'curl_slist_append',
: + 'curl_slist_free_all',
: + 'curl_strequal',
: + 'curl_strnequal',
: + 'curl_unescape',
: + 'curl_url',
: + 'curl_url_cleanup',
: + 'curl_url_dup',
: + 'curl_url_get',
: + 'curl_url_set',
: + 'curl_version',
: + 'curl_version_info',
: +}
: +
: +test:test('curl_symbols', function(t)
: + t:plan(#curl_symbols)
: + for _, sym in ipairs(curl_symbols) do
: + t:ok(
: + ffi.C.dlsym(RTLD_DEFAULT, sym) ~= nil,
: + ('Symbol %q found'):format(sym)
: + )
: + end
: +end)
: +
: +os.exit(test:check() and 0 or 1)
: --
: 2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH] Ensure all curl symbols are exported
2020-08-06 13:19 [Tarantool-patches] [PATCH] Ensure all curl symbols are exported Yaroslav Dynnikov
2020-08-06 15:06 ` Timur Safin
@ 2020-08-10 22:49 ` Vladislav Shpilevoy
2020-08-12 12:59 ` Yaroslav Dynnikov
2020-08-13 11:48 ` Kirill Yukhin
2 siblings, 1 reply; 6+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-10 22:49 UTC (permalink / raw)
To: Yaroslav Dynnikov, tarantool-patches; +Cc: Alexander Turenko
Hi! Thanks for the patch!
> diff --git a/test/box-tap/gh-5223-curl-exports.test.lua b/test/box-tap/gh-5223-curl-exports.test.lua
> new file mode 100755
> index 000000000..300d60b07
> --- /dev/null
> +++ b/test/box-tap/gh-5223-curl-exports.test.lua
> @@ -0,0 +1,177 @@
> +#!/usr/bin/env tarantool
> +
> +local tap = require('tap')
> +local ffi = require('ffi')
> +ffi.cdef([[
> + void *dlsym(void *handle, const char *symbol);
> + struct curl_version_info_data {
1. I don't think you need to copy-paste the entire structure. Anyway it is
returned from curl_version_info() by pointer, so you can just announce the
pointer type. And in the test check that ffi.C.curl_version_info(7) returned
something not nil. I don't think you need to access any fields of this
struct. It is going to lead to UB in case the struct will change anyhow in
the sources.
> + int age; /* see description below */
> + const char *version; /* human readable string */
> + unsigned int version_num; /* numeric representation */
> + const char *host; /* human readable string */
> + int features; /* bitmask, see below */
> + char *ssl_version; /* human readable string */
> + long ssl_version_num; /* not used, always zero */
> + const char *libz_version; /* human readable string */
> + const char * const *protocols; /* protocols */
> +
> + /* when 'age' is CURLVERSION_SECOND or higher, the members below exist */
> + const char *ares; /* human readable string */
> + int ares_num; /* number */
> +
> + /* when 'age' is CURLVERSION_THIRD or higher, the members below exist */
> + const char *libidn; /* human readable string */
> +
> + /* when 'age' is CURLVERSION_FOURTH or higher (>= 7.16.1), the members
> + below exist */
> + int iconv_ver_num; /* '_libiconv_version' if iconv support enabled */
> +
> + const char *libssh_version; /* human readable string */
> +
> + /* when 'age' is CURLVERSION_FIFTH or higher (>= 7.57.0), the members
> + below exist */
> + unsigned int brotli_ver_num; /* Numeric Brotli version
> + (MAJOR << 24) | (MINOR << 12) | PATCH */
> + const char *brotli_version; /* human readable string. */
> +
> + /* when 'age' is CURLVERSION_SIXTH or higher (>= 7.66.0), the members
> + below exist */
> + unsigned int nghttp2_ver_num; /* Numeric nghttp2 version
> + (MAJOR << 16) | (MINOR << 8) | PATCH */
> + const char *nghttp2_version; /* human readable string. */
> +
> + const char *quic_version; /* human readable quic (+ HTTP/3) library +
> + version or NULL */
> +
> + /* when 'age' is CURLVERSION_SEVENTH or higher (>= 7.70.0), the members
> + below exist */
> + const char *cainfo; /* the built-in default CURLOPT_CAINFO, might
> + be NULL */
> + const char *capath; /* the built-in default CURLOPT_CAPATH, might
> + be NULL */
> + };
> +
> + struct curl_version_info_data *curl_version_info(int age);
> +]])
> +
> +local info = ffi.C.curl_version_info(7)
2. Why '7'?
> +local test = tap.test('curl-features')
> +test:plan(3)
> +
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH] Ensure all curl symbols are exported
2020-08-10 22:49 ` Vladislav Shpilevoy
@ 2020-08-12 12:59 ` Yaroslav Dynnikov
2020-08-12 20:10 ` Vladislav Shpilevoy
0 siblings, 1 reply; 6+ messages in thread
From: Yaroslav Dynnikov @ 2020-08-12 12:59 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: Yaroslav Dynnikov, tml, Alexander Turenko
[-- Attachment #1: Type: text/plain, Size: 4335 bytes --]
Hi, Vlad.
Number 7 is the libcurl age. According to the documentation, age should be
set to the version of this functionality by the time you write your
program. This way, libcurl will always return a proper struct that your
program understands, while programs in the future might get a different
struct. See https://curl.haxx.se/libcurl/c/curl_version_info.html
So it's safe enough to rely on the structure. And I think both checkes
(libssl and libz) are important enough to be tracked by tests. We don't
want them to be silently gone, do we?
Best regards
Yaroslav Dynnikov
On Tue, 11 Aug 2020 at 01:49, Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
wrote:
> Hi! Thanks for the patch!
>
> > diff --git a/test/box-tap/gh-5223-curl-exports.test.lua
> b/test/box-tap/gh-5223-curl-exports.test.lua
> > new file mode 100755
> > index 000000000..300d60b07
> > --- /dev/null
> > +++ b/test/box-tap/gh-5223-curl-exports.test.lua
> > @@ -0,0 +1,177 @@
> > +#!/usr/bin/env tarantool
> > +
> > +local tap = require('tap')
> > +local ffi = require('ffi')
> > +ffi.cdef([[
> > + void *dlsym(void *handle, const char *symbol);
> > + struct curl_version_info_data {
>
> 1. I don't think you need to copy-paste the entire structure. Anyway it is
> returned from curl_version_info() by pointer, so you can just announce the
> pointer type. And in the test check that ffi.C.curl_version_info(7)
> returned
> something not nil. I don't think you need to access any fields of this
> struct. It is going to lead to UB in case the struct will change anyhow in
> the sources.
>
> > + int age; /* see description below */
> > + const char *version; /* human readable string */
> > + unsigned int version_num; /* numeric representation */
> > + const char *host; /* human readable string */
> > + int features; /* bitmask, see below */
> > + char *ssl_version; /* human readable string */
> > + long ssl_version_num; /* not used, always zero */
> > + const char *libz_version; /* human readable string */
> > + const char * const *protocols; /* protocols */
> > +
> > + /* when 'age' is CURLVERSION_SECOND or higher, the members
> below exist */
> > + const char *ares; /* human readable string */
> > + int ares_num; /* number */
> > +
> > + /* when 'age' is CURLVERSION_THIRD or higher, the members below
> exist */
> > + const char *libidn; /* human readable string */
> > +
> > + /* when 'age' is CURLVERSION_FOURTH or higher (>= 7.16.1), the
> members
> > + below exist */
> > + int iconv_ver_num; /* '_libiconv_version' if iconv
> support enabled */
> > +
> > + const char *libssh_version; /* human readable string */
> > +
> > + /* when 'age' is CURLVERSION_FIFTH or higher (>= 7.57.0), the
> members
> > + below exist */
> > + unsigned int brotli_ver_num; /* Numeric Brotli version
> > + (MAJOR << 24) | (MINOR << 12) |
> PATCH */
> > + const char *brotli_version; /* human readable string. */
> > +
> > + /* when 'age' is CURLVERSION_SIXTH or higher (>= 7.66.0), the
> members
> > + below exist */
> > + unsigned int nghttp2_ver_num; /* Numeric nghttp2 version
> > + (MAJOR << 16) | (MINOR << 8) |
> PATCH */
> > + const char *nghttp2_version; /* human readable string. */
> > +
> > + const char *quic_version; /* human readable quic (+ HTTP/3)
> library +
> > + version or NULL */
> > +
> > + /* when 'age' is CURLVERSION_SEVENTH or higher (>= 7.70.0), the
> members
> > + below exist */
> > + const char *cainfo; /* the built-in default
> CURLOPT_CAINFO, might
> > + be NULL */
> > + const char *capath; /* the built-in default
> CURLOPT_CAPATH, might
> > + be NULL */
> > + };
> > +
> > + struct curl_version_info_data *curl_version_info(int age);
> > +]])
> > +
> > +local info = ffi.C.curl_version_info(7)
>
> 2. Why '7'?
>
> > +local test = tap.test('curl-features')
> > +test:plan(3)
> > +
>
[-- Attachment #2: Type: text/html, Size: 5771 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH] Ensure all curl symbols are exported
2020-08-12 12:59 ` Yaroslav Dynnikov
@ 2020-08-12 20:10 ` Vladislav Shpilevoy
0 siblings, 0 replies; 6+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-12 20:10 UTC (permalink / raw)
To: Yaroslav Dynnikov; +Cc: tml, Alexander Turenko
On 12.08.2020 14:59, Yaroslav Dynnikov wrote:
> Hi, Vlad.
>
> Number 7 is the libcurl age. According to the documentation, age should be set to the version of this functionality by the time you write your program. This way, libcurl will always return a proper struct that your program understands, while programs in the future might get a different struct. See https://curl.haxx.se/libcurl/c/curl_version_info.html
>
> So it's safe enough to rely on the structure. And I think both checkes (libssl and libz) are important enough to be tracked by tests. We don't want them to be silently gone, do we?
Still the test is overcomplicated, I think. But at least it should be stable, if
with the same age number all curl versions will return the same curl_version_info_data.
LGTM.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH] Ensure all curl symbols are exported
2020-08-06 13:19 [Tarantool-patches] [PATCH] Ensure all curl symbols are exported Yaroslav Dynnikov
2020-08-06 15:06 ` Timur Safin
2020-08-10 22:49 ` Vladislav Shpilevoy
@ 2020-08-13 11:48 ` Kirill Yukhin
2 siblings, 0 replies; 6+ messages in thread
From: Kirill Yukhin @ 2020-08-13 11:48 UTC (permalink / raw)
To: Yaroslav Dynnikov
Cc: tarantool-patches, Vladislav Shpilevoy, Alexander Turenko
Hello,
On 06 авг 16:19, Yaroslav Dynnikov wrote:
> In the recent update of libcurl (2.5.0-278-g807c7fa58) its layout has
> changed: private function `Curl_version_init()` which used to fill-in
> info structure was eliminated. As a result, no symbols for
> `libcurl_la-version.o` remained used, so it wasn't included in tarantool
> binary. And `curl_version` and `curl_version_info` symbols went missing.
>
> According to libcurl naming conventions all exported symbols are named
> as `curl_*`. This patch lists them all explicitly in `exprots.h` and
> adds the test.
>
> Close #5223
>
> Issue: https://github.com/tarantool/tarantool/issues/5223
> Branch: https://github.com/tarantool/tarantool/tree/rosik/gh-5223-missing-curl-symbols
I've checked your patch into master.
In future, could you please append a Relase Notes entry to your patches.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-13 11:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-06 13:19 [Tarantool-patches] [PATCH] Ensure all curl symbols are exported Yaroslav Dynnikov
2020-08-06 15:06 ` Timur Safin
2020-08-10 22:49 ` Vladislav Shpilevoy
2020-08-12 12:59 ` Yaroslav Dynnikov
2020-08-12 20:10 ` Vladislav Shpilevoy
2020-08-13 11:48 ` Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox