So I've started work on converting everything over to smart pointers. You can check it out on my github here: https://github.com/rm5248/log4cxx-testing/tree/smart_pointers
Some things to note so far: - This uses a quick and dirty mechanism in autotools to switch between C++11 and boost. This is where the question of autotools vs cmake vs (something else) is important, as each build system has their own mechanism for finding and verifying that what the user wants to compile with is found. - I purged ObjectImpl completely. This was the class that was(attempting) to provide some level of smart pointers, but it was really reference counting. You had to manually do addRef/releaseRef on each object. I didn't delve too deeply into the code, but it seems like this could cause memory leaks if the references were not correctly handled. - There are a lot of methods with ObjectPtr& parameters. This is bad for smart pointers, since the internal refcount of the pointer won't get incremented(you can check the current refcount with the use_count() method) - The macros for using smart pointers instead of the pre-existing way fixed most of the pointer problems. If you're interested, the relevant lines of code are here: https://github.com/rm5248/log4cxx-testing/blob/9ed12cc2187fd81b7d75dfb79abaffb9cc1ce169/src/main/include/log4cxx/log4cxx.h.in#L47 - Some of the method signatures have to change. Because of the way smart pointers work, you can't return a polymorphic object from a method. For example, some methods were returning ObjectPtr, but changing this to shared_ptr<Object> doesn't work. See this stackoverflow question for some more information: http://stackoverflow.com/questions/196733/how-can-i-use-covariant-return-types-with-smart-pointers - Possible solution to the above problem: pas a ** pointer to a function which returns a pointer to the newly created object in this parameter instead of the return(SQLite does this when opening a new database) - Because of the above, I had to add a few dynamic_casts into the code to get it to compile(as of right now, it doesn't fully compile). A better solution here is to refactor the code to not require this, since either way this is a questionable design. The current code assumes that objects have an instanceof() method; why this was used instead of dynamic_cast I don't know, but I suspect that the reason is to avoid using RTTI information in the first place, since that is (relatively) slow. - On a lot of the files, I also haven't looked in-depth at what exactly they are doing, so some of the fixes that I am doing are educated guesses as to what I think it is trying to do. Overall, everything is going somewhat smoothly. Also note that the changes I am doing probably break API compatability, but I'm not sure how bad it will be at the moment. Given that ObjectPtr overrode operator-> and shared_ptr does as well, it seems that most of this will be transparent to other code(this is why I was able to pretty quickly change the LOG4CXX_PTR_DEF macro and have things almost work). -Robert Middleton
