Re: [Tarantool-patches] [PATCH] Ensure all curl symbols are exported

[Yaroslav Dynnikov <yaroslav.dynnikov@tarantool.org>]

[-- 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 --]