[PATCH v2] travis-ci: set right flags in release testing jobs
Alexander Turenko
alexander.turenko at tarantool.org
Mon Apr 29 15:57:06 MSK 2019
It is important to have testing jobs that build the project with both
-Werror and -O2 to keep the code clean. -O2 is needed, because some
compiler warnings are available only after extra analyzing passes that
are disabled with lesser optimization levels.
The first attempt to add -Werror for release testing jobs was made in
da505ee72883195121b7006bbd52bc66e57321f3 ('Add -Werror for CI (1.10
part)'), but it mistakely doesn't enable -O2 for RelWithDebInfoWError
build. It is possible to fix it in this way:
| --- a/cmake/compiler.cmake
| +++ b/cmake/compiler.cmake
| @@ -113,10 +113,14 @@ set (CMAKE_C_FLAGS_DEBUG
| "${CMAKE_C_FLAGS_DEBUG} ${CC_DEBUG_OPT} -O0")
| set (CMAKE_C_FLAGS_RELWITHDEBINFO
| "${CMAKE_C_FLAGS_RELWITHDEBINFO} ${CC_DEBUG_OPT} -O2")
| +set (CMAKE_C_FLAGS_RELWITHDEBINFOWERROR
| + "${CMAKE_C_FLAGS_RELWITHDEBINFOWERROR} ${CC_DEBUG_OPT} -O2")
| set (CMAKE_CXX_FLAGS_DEBUG
| "${CMAKE_CXX_FLAGS_DEBUG} ${CC_DEBUG_OPT} -O0")
| set (CMAKE_CXX_FLAGS_RELWITHDEBINFO
| "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} ${CC_DEBUG_OPT} -O2")
| +set (CMAKE_CXX_FLAGS_RELWITHDEBINFOWERROR
| + "${CMAKE_CXX_FLAGS_RELWITHDEBINFOWERROR} ${CC_DEBUG_OPT} -O2")
|
| unset(CC_DEBUG_OPT)
However I think that a build type (and so `tarantool --version`) should
not show whether -Werror was passed or not. So I have added
ENABLE_WERROR CMake option for that. It can be set like so:
| cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON
Enabled the option in testing Travis-CI jobs with the RelWithDebInfo
build type. Deploy jobs don't include it as before.
Fixed all -Wmaybe-uninitialized and -Wunused-result warnings. A few
notes about the fixes:
* net.box does not validate received data in general, so I don't add a
check for autoincrement IDs too. Set the ID to INT64_MIN, because this
value is less probably will appear here in a normal case and so is the
best one to signal a user that something probably going wrongly.
* xrow_decode_*() functions could read uninitialized data from
row->body[0].iov_base in xrow_on_decode_err() when printing a hex code
for a row. It could be possible when the received msgpack was empty
(row->bodycnt == 0), but there were expected keys (key_map != 0).
* getcwd() is marked with __attribute__((__warn_unused_result__)) in
glibc, but the buffer filled by this call is not used anywhere and so
just removed.
* Vinyl -Wmaybe-uninitialized warnings are false positive ones.
Added comments and quotes into .travis.yml to ease reading. Removed
"test" word from the CentOS 6 job name, because we don't run tests on
this distro (disabled in the RPM spec).
Fixes #4178.
---
This patch targets all currently supported long-term branches.
https://github.com/tarantool/tarantool/issues/4178
https://github.com/tarantool/tarantool/tree/Totktonada/gh-4178-set-right-flags-in-release-testing-jobs
.travis.mk | 4 ++--
.travis.yml | 45 +++++++++++++++++++++++--------------------
cmake/compiler.cmake | 16 ++++++++++-----
src/box/lua/net_box.c | 2 +-
src/box/sql/func.c | 16 +++++++--------
src/box/vy_cache.c | 2 +-
src/box/vy_mem.c | 2 +-
src/box/xrow.c | 30 ++++++++++++++++++-----------
src/lua/init.c | 2 --
9 files changed, 67 insertions(+), 52 deletions(-)
diff --git a/.travis.mk b/.travis.mk
index 247bb2a2d..973c6a7c7 100644
--- a/.travis.mk
+++ b/.travis.mk
@@ -40,7 +40,7 @@ deps_ubuntu:
lcov ruby
test_ubuntu: deps_ubuntu
- cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfoWError ${CMAKE_EXTRA_PARAMS}
+ cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
make -j8
cd test && /usr/bin/python test-run.py -j 1
@@ -51,7 +51,7 @@ deps_osx:
pip install -r test-run/requirements.txt
test_osx: deps_osx
- cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfoWError ${CMAKE_EXTRA_PARAMS}
+ cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
# Increase the maximum number of open file descriptors on macOS
sudo sysctl -w kern.maxfiles=20480 || :
sudo sysctl -w kern.maxfilesperproc=20480 || :
diff --git a/.travis.yml b/.travis.yml
index e30540837..6e79cf2fe 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -19,69 +19,72 @@ git:
jobs:
include:
- - name: RelWithDebInfoWError build + test (Linux, gcc)
+ # Testing targets (just run tests on Debian Stretch or OS X).
+ - name: "RelWithDebInfo build + test (Linux, gcc)"
env: TARGET=test
- - name: RelWithDebInfoWError build + test (Linux, clang)
+ - name: "RelWithDebInfo build + test (Linux, clang)"
env: TARGET=test
compiler: clang
- - name: RelWithDebInfoWError build + test (OS X Mojave 10.14)
+ - name: "RelWithDebInfo build + test (OS X Mojave 10.14)"
env: TARGET=test
os: osx
- - name: Debug build + test + coverage (Linux, gcc)
+ - name: "Debug build + test + coverage (Linux, gcc)"
env: TARGET=coverage
- - name: RelWithDebInfoWError build + test (OS X High Sierra 10.13)
+ - name: "RelWithDebInfo build + test (OS X High Sierra 10.13)"
env: TARGET=test
os: osx
osx_image: xcode9.4
if: branch = "master"
- - name: LTO build + test (Linux, gcc)
+ # Special targets (only LTO for now).
+ - name: "LTO build + test (Linux, gcc)"
env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON
if: branch = "master"
- - name: LTO build + test (Linux, clang)
+ - name: "LTO build + test (Linux, clang)"
env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON
if: branch = "master"
compiler: clang
- - name: LTO build + test (OS X Mojave 10.14)
+ - name: "LTO build + test (OS X Mojave 10.14)"
os: osx
env: TARGET=test CMAKE_EXTRA_PARAMS=-DENABLE_LTO=ON
if: branch = "master"
- - name: Create and deploy tarball
+ # Deploy targets (they also catch distro-specific problems).
+ - name: "Create and deploy tarball"
env: TARGET=source
if: branch = "master"
- - name: CentOS 6 build + test + deploy RPM
+ - name: "CentOS 6 build + deploy RPM"
env: OS=el DIST=6
if: branch = "master"
- - name: CentOS 7 build + test + deploy RPM
+ - name: "CentOS 7 build + test + deploy RPM"
env: OS=el DIST=7
if: branch = "master"
- - name: Fedora 28 build + test + deploy RPM
+ - name: "Fedora 28 build + test + deploy RPM"
env: OS=fedora DIST=28
if: branch = "master"
- - name: Fedora 29 build + test + deploy RPM
+ - name: "Fedora 29 build + test + deploy RPM"
env: OS=fedora DIST=29
if: branch = "master"
- - name: Ubuntu Trusty (14.04) build + deploy DEB
+ - name: "Ubuntu Trusty (14.04) build + deploy DEB"
env: OS=ubuntu DIST=trusty
if: branch = "master"
- - name: Ubuntu Xenial (16.04) build + deploy DEB
+ - name: "Ubuntu Xenial (16.04) build + deploy DEB"
env: OS=ubuntu DIST=xenial
if: branch = "master"
- - name: Ubuntu Bionic (18.04) build + deploy DEB
+ - name: "Ubuntu Bionic (18.04) build + deploy DEB"
env: OS=ubuntu DIST=bionic
if: branch = "master"
- - name: Ubuntu Cosmic (18.10) build + deploy DEB
+ - name: "Ubuntu Cosmic (18.10) build + deploy DEB"
env: OS=ubuntu DIST=cosmic
if: branch = "master"
- - name: Ubuntu Disco (19.04) build + deploy DEB
+ - name: "Ubuntu Disco (19.04) build + deploy DEB"
env: OS=ubuntu DIST=disco
if: branch = "master"
- - name: Debian Jessie (8) build + deploy DEB
+ - name: "Debian Jessie (8) build + deploy DEB"
env: OS=debian DIST=jessie
if: branch = "master"
- - name: Debian Stretch (9) build + deploy DEB
+ - name: "Debian Stretch (9) build + deploy DEB"
env: OS=debian DIST=stretch
if: branch = "master"
- - name: Debian Buster (10) build + deploy DEB
+ - name: "Debian Buster (10) build + deploy DEB"
env: OS=debian DIST=buster
if: branch = "master"
diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake
index 4062d13ec..887485c80 100644
--- a/cmake/compiler.cmake
+++ b/cmake/compiler.cmake
@@ -223,6 +223,8 @@ if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
add_definitions("-DNDEBUG" "-DNVALGRIND")
endif()
+option(ENABLE_WERROR "Make all compiler warnings into errors" OFF)
+
macro(enable_tnt_compile_flags)
# Tarantool code is written in GNU C dialect.
# Additionally, compile it with more strict flags than the rest
@@ -286,11 +288,15 @@ macro(enable_tnt_compile_flags)
add_definitions("-D__STDC_LIMIT_MACROS=1")
add_definitions("-D__STDC_CONSTANT_MACROS=1")
- # Only add -Werror if it's a Debug or RelWithDebInfoWError build, done by
- # developers. Release builds should not cause extra trouble.
- if (((${CMAKE_BUILD_TYPE} STREQUAL "Debug") OR
- (${CMAKE_BUILD_TYPE} STREQUAL "RelWithDebInfoWError")) AND
- HAVE_STD_C11 AND HAVE_STD_CXX11)
+ # Only add -Werror if it's a debug build, done by developers.
+ # Release builds should not cause extra trouble.
+ if ((${CMAKE_BUILD_TYPE} STREQUAL "Debug")
+ AND HAVE_STD_C11 AND HAVE_STD_CXX11)
+ add_compile_flags("C;CXX" "-Werror")
+ endif()
+
+ # Add -Werror if it is requested explicitly.
+ if (ENABLE_WERROR)
add_compile_flags("C;CXX" "-Werror")
endif()
endmacro(enable_tnt_compile_flags)
diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 2dfc4fc9d..7484a86bf 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -694,7 +694,7 @@ netbox_decode_sql_info(struct lua_State *L, const char **data)
assert(count > 0);
lua_createtable(L, 0, count);
for (uint32_t j = 0; j < count; ++j) {
- int64_t id;
+ int64_t id = INT64_MIN;
mp_read_int64(data, &id);
luaL_pushint64(L, id);
lua_rawseti(L, -2, j + 1);
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index f3790ba01..bb7405e68 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -758,15 +758,15 @@ sql_utf8_pattern_compare(const char *pattern,
SQL_END_OF_STRING) {
if (c == SQL_INVALID_UTF8_SYMBOL)
return INVALID_PATTERN;
- if (c != MATCH_ALL_WILDCARD &&
- c != MATCH_ONE_WILDCARD)
+ if (c == MATCH_ONE_WILDCARD) {
+ c2 = Utf8Read(string, string_end);
+ if (c2 == SQL_INVALID_UTF8_SYMBOL)
+ return NO_MATCH;
+ if (c2 == SQL_END_OF_STRING)
+ return NO_WILDCARD_MATCH;
+ } else if (c != MATCH_ALL_WILDCARD) {
break;
- if (c == MATCH_ONE_WILDCARD &&
- (c2 = Utf8Read(string, string_end)) ==
- SQL_END_OF_STRING)
- return NO_WILDCARD_MATCH;
- if (c2 == SQL_INVALID_UTF8_SYMBOL)
- return NO_MATCH;
+ }
}
/*
* "%" at the end of the pattern matches.
diff --git a/src/box/vy_cache.c b/src/box/vy_cache.c
index be11b986f..7007d0e35 100644
--- a/src/box/vy_cache.c
+++ b/src/box/vy_cache.c
@@ -661,7 +661,7 @@ vy_cache_iterator_seek(struct vy_cache_iterator *itr, struct vy_entry last)
ITER_GT : ITER_LT;
}
- bool exact;
+ bool exact = false;
if (!vy_stmt_is_empty_key(key.stmt)) {
itr->curr_pos = iterator_type == ITER_EQ ||
iterator_type == ITER_GE ||
diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c
index 9513d5b80..a4fae26e2 100644
--- a/src/box/vy_mem.c
+++ b/src/box/vy_mem.c
@@ -392,7 +392,7 @@ vy_mem_iterator_seek(struct vy_mem_iterator *itr, struct vy_entry last)
ITER_GT : ITER_LT;
}
- bool exact;
+ bool exact = false;
struct vy_mem_tree_key tree_key;
tree_key.entry = key;
/* (lsn == INT64_MAX - 1) means that lsn is ignored in comparison */
diff --git a/src/box/xrow.c b/src/box/xrow.c
index aaed84e38..a3c9b3218 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -509,9 +509,12 @@ iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
schema_version, sizeof(body) + msg_len);
body.v_data_len = mp_bswap_u32(msg_len);
- (void) write(fd, header, sizeof(header));
- (void) write(fd, &body, sizeof(body));
- (void) write(fd, e->errmsg, msg_len);
+
+ ssize_t unused;
+ unused = write(fd, header, sizeof(header));
+ unused = write(fd, &body, sizeof(body));
+ unused = write(fd, e->errmsg, msg_len);
+ (void) unused;
}
int
@@ -624,12 +627,15 @@ xrow_decode_dml(struct xrow_header *row, struct request *request,
request->header = row;
request->type = row->type;
+ const char *start = NULL;
+ const char *end = NULL;
+
if (row->bodycnt == 0)
goto done;
assert(row->bodycnt == 1);
- const char *data = (const char *) row->body[0].iov_base;
- const char *end = data + row->body[0].iov_len;
+ const char *data = start = (const char *) row->body[0].iov_base;
+ end = data + row->body[0].iov_len;
assert((end - data) > 0);
if (mp_typeof(*data) != MP_MAP || mp_check_map(data, end) > 0) {
@@ -701,8 +707,8 @@ error:
done:
if (key_map) {
enum iproto_key key = (enum iproto_key) bit_ctz_u64(key_map);
- xrow_on_decode_err(row->body[0].iov_base, end,
- ER_MISSING_REQUEST_FIELD, iproto_key_name(key));
+ xrow_on_decode_err(start, end, ER_MISSING_REQUEST_FIELD,
+ iproto_key_name(key));
return -1;
}
return 0;
@@ -1071,12 +1077,15 @@ xrow_decode_ballot(struct xrow_header *row, struct ballot *ballot)
ballot->is_ro = false;
vclock_create(&ballot->vclock);
+ const char *start = NULL;
+ const char *end = NULL;
+
if (row->bodycnt == 0)
goto err;
assert(row->bodycnt == 1);
- const char *data = (const char *) row->body[0].iov_base;
- const char *end = data + row->body[0].iov_len;
+ const char *data = start = (const char *) row->body[0].iov_base;
+ end = data + row->body[0].iov_len;
const char *tmp = data;
if (mp_check(&tmp, end) != 0 || mp_typeof(*data) != MP_MAP)
goto err;
@@ -1124,8 +1133,7 @@ xrow_decode_ballot(struct xrow_header *row, struct ballot *ballot)
}
return 0;
err:
- xrow_on_decode_err(row->body[0].iov_base, end, ER_INVALID_MSGPACK,
- "packet body");
+ xrow_on_decode_err(start, end, ER_INVALID_MSGPACK, "packet body");
return -1;
}
diff --git a/src/lua/init.c b/src/lua/init.c
index d18c8af94..be164bdfb 100644
--- a/src/lua/init.c
+++ b/src/lua/init.c
@@ -318,8 +318,6 @@ static void
tarantool_lua_setpaths(struct lua_State *L)
{
const char *home = getenv("HOME");
- char cwd[PATH_MAX] = {'\0'};
- getcwd(cwd, sizeof(cwd));
lua_getglobal(L, "package");
int top = lua_gettop(L);
--
2.21.0
More information about the Tarantool-patches
mailing list