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 63E8329B2B for ; Thu, 29 Mar 2018 14:33:03 -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 gJTSlJZGFjfd for ; Thu, 29 Mar 2018 14:33:03 -0400 (EDT) Received: from smtp5.mail.ru (smtp5.mail.ru [94.100.179.24]) (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 0966A29B22 for ; Thu, 29 Mar 2018 14:33:02 -0400 (EDT) Date: Thu, 29 Mar 2018 21:33:07 +0300 From: Alexander Turenko Subject: [tarantool-patches] Re: [PATCH] Fix warnings. Message-ID: <20180329183306.5ywa5akot35tgo7u@tkn_work_nb> References: <1522247382-8505-1-git-send-email-gleb-skiba@mail.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1522247382-8505-1-git-send-email-gleb-skiba@mail.ru> 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: Gleb Cc: tarantool-patches@freelists.org 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 */ > char *zRight = 0; /* Nul-terminated UTF-8 string , or NULL */ > char *zTable = 0; /* Nul-terminated UTF-8 string 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 >