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 9079E2BB1C for ; Sun, 28 Apr 2019 17:38:16 -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 zjPq2q3MkSVl for ; Sun, 28 Apr 2019 17:38:16 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 504052B420 for ; Sun, 28 Apr 2019 17:38:16 -0400 (EDT) Date: Mon, 29 Apr 2019 00:38:04 +0300 From: Alexander Turenko Subject: [tarantool-patches] Re: [PATCH] travis-ci: set right flags in release testing jobs Message-ID: <20190428213804.gpkzhpz6j3y6drfw@tkn_work_nb> References: <64db98703d236c2e6775601a48fb19c3bc8ba955.1556241512.git.alexander.turenko@tarantool.org> <20190428054028.vxdwsboswt5strjl@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: 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: Vladislav Shpilevoy Cc: tarantool-patches@freelists.org On Sun, Apr 28, 2019 at 12:11:28PM +0300, Vladislav Shpilevoy wrote: > > >> > >>> * xrow_decode_*() functions could read uninitialized data when printing > >>> a hex code for a row if the received msgpack is empty, but there are > >>> expected keys. > >> > >> Please, suggest a change for wording if the current one is not clear. > > Ahh, now I see. It is possible that row->bodycnt == 0 and we jump to > 'done'. It tries to write an error via xrow_on_decode_err, which reads > row->body[0]. Probably then I can suggest adding an explicit mention of > xrow_on_decode_err() function and an attempt to access an array by index > out of range. > > I did not understand your explanation below about cycles and > dump_row_hex(), because I did not know, that they are not used in > xrow_decode_dml() directly, but via xrow_on_decode_err(). > > >> My change causes `for (const char *cur = start; cur < end;) { ... }` in > >> dump_row_hex() to walk around the loop body and so prevents reading from > >> `start` in the loop (NULL < NULL is false). > > Then you can ignore my comments about xrow_decode_dml and xrow_decode_ballot. > Others are still valid. I have updated the comment about the uninitialized data read, added the comment about getcwd() and the comment about Vinyl: > 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. If the patch is okay for you, I'll resend it to Vladimir D. as v2. > By the way, > > \\\ , > \ `| > ) ( .-""-. > | | /_ { '. > | | (/ `\ } ) > | | ^/ ^`} { > \ \ \= ( { ) > \ \ '-, { {{ > \ \_.' ) } ) > \.-' ( ( > /'-.'_. ) ( } > \_( { _/\ > ) '--' `-;\ \ > _.-' / / / > <\/>_.' .' / / > <\/>/. ' /<\// / > _ |\`- _ . -/| <\/> - _- ` _.-'`_/- | \ > - - - - \\\ > }`<\/> <\/>`{ > { -<\/>_<\/>_<\/>- } > } { > <\/>. <\/> > > {`<\/> <\/>`} > } -<\/>_<\/>_<\/>_<\/>- { > { } > } } > { H A P P Y { > <\/> B I R T H D A Y <\/> > > `<\/> <\/>' > -<\/>_<\/>_<\/>_<\/>_<\/>- > Thanks!