[tarantool-patches] Re: [PATCH] Fix warnings.
Alexander Turenko
alexander.turenko at tarantool.org
Thu Mar 29 21:33:07 MSK 2018
Hi, Gleb!
Consider comments below.
I propose to add another release target with -Werror, consider [g].
[g]: https://github.com/tarantool/tarantool/issues/3238#issuecomment-377319953
WBR, Alexander Turenko.
On Wed, Mar 28, 2018 at 05:29:42PM +0300, Gleb wrote:
> Ensure -Werror -Wall set for the whole src/.
> Fix warnings which have been find with prev flags.
>
> Fixes #3238
Please, remove the dot at the end of the commit title.
> ---
> -Issue from https://github.com/tarantool/tarantool/issues/3238.
> -Source from https://github.com/tarantool/tarantool/tree/gh-3238-check-warnings.
> src/box/sql/alter.c | 2 +-
> src/box/sql/pragma.c | 3 +--
> src/box/vy_read_iterator.c | 2 +-
> src/box/xrow.c | 10 ++++++----
> src/httpc.c | 2 +-
> src/lua/init.c | 2 +-
> src/say.c | 6 ++++--
> 7 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
> index a52ba8d..098d183 100644
> --- a/src/box/sql/alter.c
> +++ b/src/box/sql/alter.c
> @@ -150,7 +150,7 @@ sqlite3AlterFinishAddColumn(Parse * pParse, Token * pColDef)
> Column *pCol; /* The new column */
> Expr *pDflt; /* Default value for the new column */
> sqlite3 *db; /* The database connection; */
> - Vdbe *v = pParse->pVdbe; /* The prepared statement under construction */
> + Vdbe *v __attribute__((unused)) = pParse->pVdbe; /* The prepared statement under construction */
> struct session *user_session = current_session();
>
The line is longer that 80 symbols.
Please, use portable macro MAYBE_UNUSED from src/trivia/util.h (or cast
to void as an separate statement).
> db = pParse->db;
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index 23b4c73..4ecc18d 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -257,7 +257,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
> char *zLeft = 0; /* Nul-terminated UTF-8 string <id> */
> char *zRight = 0; /* Nul-terminated UTF-8 string <value>, or NULL */
> char *zTable = 0; /* Nul-terminated UTF-8 string <value2> or NULL */
> - int rc; /* return value form SQLITE_FCNTL_PRAGMA */
> sqlite3 *db = pParse->db; /* The database connection */
> Vdbe *v = sqlite3GetVdbe(pParse); /* Prepared statement */
> const PragmaName *pPragma; /* The pragma */
> @@ -521,7 +520,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
> box_tuple_t *tuple;
> box_iterator_t* iter;
> iter = box_index_iterator(space_id, 0,ITER_ALL, key_buf, key_end);
> - rc = box_iterator_next(iter, &tuple);
> + int rc __attribute__((unused)) = box_iterator_next(iter, &tuple); /* return value form SQLITE_FCNTL_PRAGMA */
Long line.
The comment seems to be unrelated, it is better to remove it.
> assert(rc==0);
> for (i = 0; tuple!=NULL; i++, box_iterator_next(iter, &tuple)){
> /* 1 is name field number */
> diff --git a/src/box/vy_read_iterator.c b/src/box/vy_read_iterator.c
> index a265f58..93681a8 100644
> --- a/src/box/vy_read_iterator.c
> +++ b/src/box/vy_read_iterator.c
> @@ -552,7 +552,7 @@ vy_read_iterator_next_lsn(struct vy_read_iterator *itr, struct tuple **ret)
> {
> uint32_t i;
> bool unused;
> - struct vy_read_src *src;
> + struct vy_read_src *src = NULL;
>
> assert(itr->curr_stmt != NULL);
> assert(itr->curr_src < itr->skipped_src);
> diff --git a/src/box/xrow.c b/src/box/xrow.c
> index 3ef3d82..b272166 100644
> --- a/src/box/xrow.c
> +++ b/src/box/xrow.c
> @@ -300,7 +300,7 @@ iproto_reply_vclock(struct obuf *out, uint64_t sync, uint32_t schema_version,
> iproto_header_encode(buf, IPROTO_OK, sync, schema_version,
> size - IPROTO_HEADER_LEN);
>
> - char *ptr = obuf_alloc(out, size);
> + char *ptr __attribute__((unused)) = obuf_alloc(out, size);
> assert(ptr == buf);
> return 0;
> }
> @@ -339,9 +339,11 @@ 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 r;
> + r = write(fd, header, sizeof(header));
> + r = write(fd, &body, sizeof(body));
> + r = write(fd, e->errmsg, msg_len);
> + (void) r;
write has __wur flag for which cast-to-void broken at times. The story
is in [1], [2] and [3]. So this is workaround for gcc bug, okay.
I afraid that 1st and 2nd *values* of `r` are ignored and gcc can give
warning about it in the future. But I'm okay if it works good from gcc
4.5 to gcc 7.2.
[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
[2]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509
[3]: https://sourceware.org/bugzilla/show_bug.cgi?id=11959
> }
>
> int
> diff --git a/src/httpc.c b/src/httpc.c
> index 633e688..d6c1b00 100644
> --- a/src/httpc.c
> +++ b/src/httpc.c
> @@ -249,7 +249,7 @@ httpc_set_low_speed_limit(struct httpc_request *req, long low_speed_limit)
> void
> httpc_set_verbose(struct httpc_request *req, bool curl_verbose)
> {
> - curl_easy_setopt(req->curl_request.easy, CURLOPT_VERBOSE, curl_verbose);
> + curl_easy_setopt(req->curl_request.easy, CURLOPT_VERBOSE, (long)curl_verbose);
> }
>
Too long line (tab is 8 symbols long), please, break it into parts.
Separate cast operator from an expression by a space.
> void
> diff --git a/src/lua/init.c b/src/lua/init.c
> index 76e978c..84f81e6 100644
> --- a/src/lua/init.c
> +++ b/src/lua/init.c
> @@ -279,7 +279,7 @@ tarantool_lua_setpaths(struct lua_State *L)
> {
> const char *home = getenv("HOME");
> char cwd[PATH_MAX] = {'\0'};
> - getcwd(cwd, sizeof(cwd));
> + char *buf __attribute__((unused)) = getcwd(cwd, sizeof(cwd));
> lua_getglobal(L, "package");
> int top = lua_gettop(L);
>
> diff --git a/src/say.c b/src/say.c
> index b92514d..0801f04 100644
> --- a/src/say.c
> +++ b/src/say.c
> @@ -1040,8 +1040,10 @@ log_vsay(struct log *log, int level, const char *filename, int line,
> break;
> case SAY_LOGGER_SYSLOG:
> write_to_syslog(log, total);
> - if (level == S_FATAL && log->fd != STDERR_FILENO)
> - (void) write(STDERR_FILENO, buf, total);
> + if (level == S_FATAL && log->fd != STDERR_FILENO) {
> + ssize_t r = write(STDERR_FILENO, buf, total);
> + (void) r; /* silence gcc warning */
Why do you fill 23 spaces before the comment? Maybe just 1 is better?
> + }
> break;
> case SAY_LOGGER_BOOT
> {
> --
> 2.7.4
>
More information about the Tarantool-patches
mailing list