Resource leak, and other warnings from Coverity

[krlmlr]

When analyzing RMariaDB, a warning most likely related to Rcpp came up.

Source code location:

Rcpp/inst/include/Rcpp/iostream/Rstreambuf.h

Line 49 in f9b700a

std::ostream( new Buffer ),

Problem: The new Buffer seems to be never freed. This might be a false positive, but maybe not.

Screenshot (because Coverity reports are private):

The other warning is about a constructor that doesn't initialize class members in tinyformat.h:

Rcpp/inst/include/Rcpp/utils/tinyformat.h

Line 497 in f9b700a

FormatArg() {}

Screenshot:

I've ignored the exception warnings so far, but we may take a look at them later.

[eddelbuettel]

I'm not sure. Just look at the handful of lines at

Rcpp/inst/include/Rcpp/iostream/Rstreambuf.h

Lines 43 to 58 in f9b700a

	template <bool OUTPUT>
	class Rostream : public std::ostream {
	typedef Rstreambuf<OUTPUT> Buffer ;
	Buffer* buf ;
	public:
	Rostream() :
	std::ostream( new Buffer ),
	buf( static_cast<Buffer*>( rdbuf() ) )
	{}
	~Rostream() {
	if (buf != NULL) {
	delete buf;
	buf = NULL;
	}
	}
	};

Sure, the delete is conditional, but it is still there. One layer of typedef should not confuse.

Suggest to close this.

PS Congrats on being your own boss now!

[kevinushey]

FWIW we use a similar pattern in testthat:

https://github.com/hadley/testthat/blob/master/inst/include/testthat/testthat.h#L92-L96

Do you see the same warning there? I think the code as-is in Rcpp should be fine as well though. (Presumedly, if it weren't, valgrind and other tools would report a memory leak when exiting R after loading Rcpp)

[krlmlr]

@eddelbuettel: Thanks, also for the congrats. The code is fine, but marginally confusing, both to humans and to static code analyzers. Maybe declare Buffer buf instead of Buffer* buf? This doesn't seem to introduce extra cohesion, because the buffer class is defined just above the stream class.

@kevinushey: Haven't tested testthat yet.

What about the empty FormatArg() constructor?

[eddelbuettel]

What about the empty FormatArg() constructor?

That's third party code (from tinyformat). First order of biz would be to see if it changed.

[krlmlr]

Submitted upstream. Happy to provide a PR that makes buf a class member instead of a pointer member.

[eddelbuettel]

Sure, if you can re-test with coverity and rephrase the code so that we don't get yelled at ...