I think going for a boost::shared_ptr could be a good idea, most
projects using something as complex as log4cxx should be able to cope
with it imho. Another option would be to use some sort of ifndef rule to
determine if boost or the current implementation is used.
For levels though, I dont really see why we even have smart pointers? We
could just as easily create the variables statically in the class, and
use them as const references to the original levels. Right now the
library is allocating new dynamic memory on a call basis, which seems
pretty wasteful to me. If we make the storage static (which it was prior
to LOG4CXX-394/r1566655 anyway). In some cases like the Logger class
that stores a changeable level it would have to be a plain pointer, but
I personally dont see a problem with that.
/Peter
On 10/27/2016 08:48 AM, Thorsten Schöning wrote:
Guten Tag Robert Middleton,
am Donnerstag, 27. Oktober 2016 um 01:39 schrieben Sie:
I haven't delved too deeply into the code(I don't have time today),
but it looks like all that Object is doing is providing Java-like
garbage collection in terms of memory management.
That was my understanding as well, but than Valgrind shouldn't find
leaks?
A quick look at
the SVN history indicates that the class was first created ~2003, so
would it be worth it to discard Object entirely in favor of C++11
std::shared_ptr?
And what with all those non C++11 people out there? I'm using
Embarcadero C++Builder for example which doesn't support C++11 very
well with its legacy compiler and the new CLANG based ones are still
incompatible with some projects/environments. So at least I'm not that
interested to merge patches which make my life more difficult...
How about boost::shared_ptr instead? It's no requirement currently,
but one might argue it's not that different than e.g. apr. In my
compiler at least boost 1.39 is provided, which would be enough for
shared_ptr, I'm using those already. 1.39 itself is so old, that it
might be a reasonable minimum requirement.
Mit freundlichen Grüßen,
Thorsten Schöning
