From: Alexander Turenko <alexander.turenko@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH] travis-ci: set right flags in release testing jobs Date: Mon, 29 Apr 2019 00:38:04 +0300 [thread overview] Message-ID: <20190428213804.gpkzhpz6j3y6drfw@tkn_work_nb> (raw) In-Reply-To: <b03bc975-3091-bf78-90d8-aa02d2dee9fb@tarantool.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!
next prev parent reply other threads:[~2019-04-28 21:38 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-26 1:29 [tarantool-patches] " Alexander Turenko 2019-04-26 10:18 ` [tarantool-patches] " Vladislav Shpilevoy 2019-04-28 5:40 ` Alexander Turenko 2019-04-28 8:54 ` Vladislav Shpilevoy 2019-04-28 9:11 ` Vladislav Shpilevoy 2019-04-28 21:38 ` Alexander Turenko [this message] 2019-04-28 21:35 ` Alexander Turenko 2019-04-29 10:07 ` Vladislav Shpilevoy 2019-04-29 12:57 ` [PATCH v2] " Alexander Turenko 2019-04-29 17:09 ` Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190428213804.gpkzhpz6j3y6drfw@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='[tarantool-patches] Re: [PATCH] travis-ci: set right flags in release testing jobs' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox