From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 8A671282C1 for ; Mon, 13 Aug 2018 00:21:27 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YI1tCxKJJg-I for ; Mon, 13 Aug 2018 00:21:27 -0400 (EDT) Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id AAAA728247 for ; Mon, 13 Aug 2018 00:21:26 -0400 (EDT) From: Georgy Kirichenko Subject: [tarantool-patches] Re: [PATCH v2] add FindICONV and iconv wrapper Date: Mon, 13 Aug 2018 07:21:23 +0300 Message-ID: <1995951.6Ft1TRgIS4@home.lan> In-Reply-To: <20180801154931.52723-1-k.belyavskiy@tarantool.org> References: <20180801154931.52723-1-k.belyavskiy@tarantool.org> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart2459424.UYytv4IazM"; micalg="pgp-sha256"; protocol="application/pgp-signature" Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Konstantin Belyavskiy --nextPart2459424.UYytv4IazM Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" LGTM, thanks On Wednesday, August 1, 2018 6:49:31 PM MSK Konstantin Belyavskiy wrote: > Fixing build under FreeBSD: Undefined symbol "iconv_open" > Add compile time build check with FindICONV.cmake > and a wrapper to import relevant symbol names with include file. > > Changes in V2: > - update wrong comment > - rename file and functions iconv_wrap* to tnt_iconv* > - remove unnecessary include file > > Resend branch from 23/07 since it was a reply with request to > attach a patch to a mail and then further discussion stops. > > Closes #3441 > --- > ticket: https://github.com/tarantool/tarantool/issues/3441 > branch: > https://github.com/tarantool/tarantool/tree/kbelyavs/gh-3441-fix-linking-wi > th-iconv > > CMakeLists.txt | 6 ++++++ > cmake/FindICONV.cmake | 44 ++++++++++++++++++++++++++++++++++++++++++++ > extra/exports | 3 +++ > src/CMakeLists.txt | 9 +++------ > src/lua/iconv.lua | 41 +++++++++-------------------------------- > src/lua/tnt_iconv.c | 21 +++++++++++++++++++++ > 6 files changed, 86 insertions(+), 38 deletions(-) > create mode 100644 cmake/FindICONV.cmake > create mode 100644 src/lua/tnt_iconv.c > > diff --git a/CMakeLists.txt b/CMakeLists.txt > index d0cae0f01..cea4a7d3e 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -299,6 +299,12 @@ find_package(CURL) > set(Readline_FIND_REQUIRED ON) > find_package(Readline) > > +# > +# ICONV > +# > +set(ICONV_FIND_REQUIRED ON) > +find_package(ICONV) > + > # > # ICU > # > diff --git a/cmake/FindICONV.cmake b/cmake/FindICONV.cmake > new file mode 100644 > index 000000000..2ae9c8197 > --- /dev/null > +++ b/cmake/FindICONV.cmake > @@ -0,0 +1,44 @@ > +# - Find the iconv include file and optional library > +# > +# ICONV_INCLUDE_DIRS > +# ICONV_LIBRARIES > +# > + > +if (TARGET_OS_LINUX) > + set(ICONV_LIBRARY "") > +else() > + find_library(ICONV_LIBRARY iconv) > + if(NOT ICONV_LIBRARY) > + message(WARNING "iconv library not found") > + set(ICONV_LIBRARY "") > + endif() > +endif() > +find_path(ICONV_INCLUDE_DIR iconv.h) > +if(NOT ICONV_INCLUDE_DIR) > + message(FATAL_ERROR "iconv include header not found") > +endif() > +set(ICONV_LIBRARIES ${ICONV_LIBRARY}) > +set(ICONV_INCLUDE_DIRS ${ICONV_INCLUDE_DIR}) > +set(CMAKE_REQUIRED_LIBRARIES ${ICONV_LIBRARIES}) > +set(CMAKE_REQUIRED_INCLUDES ${ICONV_INCLUDE_DIRS}) > +check_c_source_runs(" > + #include > + int main() > + { > + void* foo = iconv_open(\"UTF-8\", \"ISO-8859-1\"); > + iconv_close(foo); > + return 0; > + } > + " ICONV_RUNS) > +set(CMAKE_REQUIRED_LIBRARIES "") > +set(CMAKE_REQUIRED_INCLUDES "") > +if (NOT DEFINED ICONV_RUNS_EXITCODE OR ICONV_RUNS_EXITCODE) > + unset(ICONV_LIBRARIES) > + unset(ICONV_INCLUDE_DIRS) > + set(ICONV_FOUND false) > + if (ICONV_FIND_REQUIRED) > + message(FATAL_ERROR "ICONV does not run") > + endif() > +endif() > + > +mark_as_advanced(ICONV_INCLUDE_DIRS ICONV_LIBRARIES) > diff --git a/extra/exports b/extra/exports > index 61a2e0a54..490118e36 100644 > --- a/extra/exports > +++ b/extra/exports > @@ -38,6 +38,9 @@ title_set_custom > title_get_custom > title_set_status > title_get_status > +tnt_iconv_open > +tnt_iconv_close > +tnt_iconv > exception_get_string > exception_get_int > > diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt > index 8ab09e968..b1c4cd711 100644 > --- a/src/CMakeLists.txt > +++ b/src/CMakeLists.txt > @@ -12,6 +12,7 @@ include_directories(${LIBYAML_INCLUDE_DIRS}) > include_directories(${MSGPUCK_INCLUDE_DIRS}) > include_directories(${CURL_INCLUDE_DIRS}) > include_directories(${ICU_INCLUDE_DIRS}) > +include_directories(${ICONV_INCLUDE_DIRS}) > > set(LIBUTIL_FREEBSD_SRC ${CMAKE_SOURCE_DIR}/third_party/libutil_freebsd) > include_directories(${LIBUTIL_FREEBSD_SRC}) > @@ -159,6 +160,7 @@ set (server_sources > lua/msgpack.c > lua/utils.c > lua/errno.c > + lua/tnt_iconv.c > lua/socket.c > lua/pickle.c > lua/fio.c > @@ -221,6 +223,7 @@ set (common_libraries > ${READLINE_LIBRARIES} > ${OPENSSL_LIBRARIES} > ${CURL_LIBRARIES} > + ${ICONV_LIBRARIES} > ) > > if (TARGET_OS_LINUX OR TARGET_OS_DEBIAN_FREEBSD) > @@ -234,12 +237,6 @@ if (TARGET_OS_FREEBSD AND NOT TARGET_OS_DEBIAN_FREEBSD) > else() > set (common_libraries ${common_libraries} ${INTL}) > endif() > - find_library (ICONV iconv) > - if (NOT ICONV) > - message(FATAL_ERROR "iconv library not found") > - else() > - set (common_libraries ${common_libraries} ${ICONV}) > - endif() > endif() > > set (common_libraries ${common_libraries} ${LIBUUID_LIBRARIES}) > diff --git a/src/lua/iconv.lua b/src/lua/iconv.lua > index 6a6bfeb76..ade66d760 100644 > --- a/src/lua/iconv.lua > +++ b/src/lua/iconv.lua > @@ -4,17 +4,10 @@ local buffer = require('buffer') > > ffi.cdef[[ > typedef struct iconv *iconv_t; > -iconv_t iconv_open(const char *tocode, const char *fromcode); > -void iconv_close(iconv_t cd); > -size_t iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, > - char **outbuf, size_t *outbytesleft); > -/* > - * add prefix 'lib' under FreeBSD > - */ > -iconv_t libiconv_open(const char *tocode, const char *fromcode); > -void libiconv_close(iconv_t cd); > -size_t libiconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, > - char **outbuf, size_t *outbytesleft); > +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); > ]] > > local iconv_t = ffi.typeof('struct iconv') > @@ -23,22 +16,6 @@ local cchar_ptr_arr_t = ffi.typeof('const char *[1]') > local cchar_ptr_t = ffi.typeof('const char *') > local size_t_arr_t = ffi.typeof('size_t [1]') > > -local _iconv_open > -local _iconv_close > -local _iconv > - > --- To fix #3073, BSD iconv implementation is not fully > --- compatible with iconv, so use external iconv.so lib > -if jit.os == 'BSD' then > - _iconv_open = ffi.C.libiconv_open > - _iconv_close = ffi.C.libiconv_close > - _iconv = ffi.C.libiconv > -else > - _iconv_open = ffi.C.iconv_open > - _iconv_close = ffi.C.iconv_close > - _iconv = ffi.C.iconv > -end > - > local E2BIG = errno['E2BIG'] > local EINVAL = errno['EINVAL'] > local EILSEQ = errno['EILSEQ'] > @@ -64,10 +41,10 @@ local function iconv_convert(iconv, data) > while data_left[0] > 0 do > buf_ptr[0] = buf:reserve(output_len) > buf_left[0] = buf:unused() > - local res = _iconv(iconv, data_ptr, data_left, > + local res = ffi.C.tnt_iconv(iconv, data_ptr, data_left, > buf_ptr, buf_left) > if res == ffi.cast('size_t', -1) and errno() ~= E2BIG then > - _iconv(iconv, nil, nil, nil, nil) > + ffi.C.tnt_iconv(iconv, nil, nil, nil, nil) > if errno() == EINVAL then > error('Invalid multibyte sequence') > end > @@ -80,7 +57,7 @@ local function iconv_convert(iconv, data) > end > > -- iconv function sets cd's conversion state to the initial state > - _iconv(iconv, nil, nil, nil, nil) > + ffi.C.tnt_iconv(iconv, nil, nil, nil, nil) > local result = ffi.string(buf.rpos, buf:size()) > buf:reset() > return result > @@ -88,7 +65,7 @@ end > > local iconv_mt = { > __call = iconv_convert, > - __gc = _iconv_close, > + __gc = ffi.C.tnt_iconv_close, > __tostring = function(iconv) return string.format("iconv: %p", iconv) > end } > > @@ -98,7 +75,7 @@ local function iconv_new(to, from) > if type(to) ~= 'string' or type(from) ~= 'string' then > error('Usage: iconv.new("CP1251", "KOI8-R")') > end > - local iconv = _iconv_open(to, from) > + local iconv = ffi.C.tnt_iconv_open(to, from) > if iconv == conv_rv_error then > error('iconv: '..errno.strerror()) > end > diff --git a/src/lua/tnt_iconv.c b/src/lua/tnt_iconv.c > new file mode 100644 > index 000000000..c91fef140 > --- /dev/null > +++ b/src/lua/tnt_iconv.c > @@ -0,0 +1,21 @@ > +#include > + > +iconv_t > +tnt_iconv_open(const char *tocode, const char *fromcode) > +{ > + return iconv_open(tocode, fromcode); > +} > + > +int > +tnt_iconv_close(iconv_t cd) > +{ > + return iconv_close(cd); > +} > + > +size_t > +tnt_iconv(iconv_t cd, char **inbuf, size_t *inbytesleft, > + char **outbuf, size_t *outbytesleft) > +{ > + return iconv(cd, inbuf, inbytesleft, > + outbuf, outbytesleft); > +} --nextPart2459424.UYytv4IazM Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEBJFDbU76LsBbgHBsvKOmCX79zb4FAltxB0MACgkQvKOmCX79 zb7/Hgf/Tfn4uUUNJASuBPGgLOIlsN5SJ6vKx/SKlMBCUVJx035fWTXv835dXEph fw26Di2Zo5k7z3h13/K/206hEWfnGZGrJt2hRp4s8GCWag46n7QpPoUSSLj6qmR9 aSzbqqwujj1rF3i15E6aK3HhrMpdLUdCgpnc/4bPSJKMJyXCk3Vygll10ocLTRIw m4a+dISCqcsAablP/i/hq/lXfSkTQ27pilBIob5Zq779IiiASKQu2C1lf5WZ92SG OiV7Me0pydBNAz0JEjf9cXhr5Ylsp7Sh4ffnctLehemr0ziTIpmqQxwOHpIOvTbT 1TqsV83tdxG4GvtA3j6ZSxgpJnpTYQ== =JNOl -----END PGP SIGNATURE----- --nextPart2459424.UYytv4IazM--