Re: [Tarantool-patches] [PATCH v4 2/2] box: introduce _vsession_settings sysview

[Mergen Imeev <imeevma@tarantool.org>]

Thank you for review! My answer and diff between patches
below.

On Sun, Nov 24, 2019 at 04:27:58PM +0100, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> > diff --git a/src/box/session.cc b/src/box/session.cc
> > index 461d1cf..93d8b42 100644
> > --- a/src/box/session.cc
> > +++ b/src/box/session.cc
> > @@ -338,3 +341,131 @@ generic_session_sync(struct session *session)
> > +
> > +struct iterator *
> > +session_options_create_iterator(struct index *index, enum iterator_type type,
> > +				const char *key, uint32_t part_count)
> > +{
> > +	char *decoded_key = NULL;
> > +	if (part_count > 0) {
> > +		assert(part_count == 1);
> > +		assert(mp_typeof(*key) == MP_STR);
> > +		uint32_t len;
> > +		const char *name = mp_decode_str(&key, &len);
> > +		decoded_key = (char *)malloc(len + 1);
> > +		if (decoded_key == NULL) {
> > +			diag_set(OutOfMemory, len + 1, "malloc", "decoded_key");
> > +			return NULL;
> > +		}
> > +		memcpy(decoded_key, name, len);
> > +		decoded_key[len] = '\0';
> > +	}
> > +	struct space *space = space_cache_find(index->def->space_id);
> > +	struct session_options_iterator *it =
> > +		(session_options_iterator *)malloc(sizeof(*it));
> > +	if (it == NULL) {
> 
> decoded_key leaks here. But heap OOM is such an unlikely event,
> and is forgotten for be checked so often, that perhaps we should
> reconsider moving this https://github.com/tarantool/tarantool/issues/3534
> out of wish list and start using panic in new places. Probably
> wrapped into a 'heap_alloc()' function (name is discussable).
> 
> I think, Kirill won't be against this.
Fixed. I asked about issue #3534 in a scrum, but haven’t
received a definite answer yet. I will ask about it again,
but a little later, since now we have to prepare for
release 2.3.1. Also, it looks like we should replace all
malloc() checks in one commit.

diff --git a/src/box/session.cc b/src/box/session.cc
index 93d8b42..fbaabf0 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -454,6 +454,7 @@ session_options_create_iterator(struct index *index, enum iterator_type type,
 		(session_options_iterator *)malloc(sizeof(*it));
 	if (it == NULL) {
 		diag_set(OutOfMemory, sizeof(*it), "malloc", "it");
+		free(decoded_key);
 		return NULL;
 	}
 	int current_id = iterator_type_is_reverse(type) ?