Success! log4cxx now compiles, but it almost certainly doesn't work yet. I need to do some testing on it.
Things that need to be changed: - Figure out what classes/methods used to take an ObjectPtr&. If they should actually own this object in any way, change this to not pass by reference so that the smart pointer refcount actually gets incremented. It seems that log4cxx works as long as objects don't get deconstructed. Since the internal pointer refcount of shared_ptr doesn't get incremented in this case, the method will be called, the smart pointer goes out of scope in the calling method, and then I expect that the process will get a SIGSEGV. - Some methods have been changed to pass a raw pointer to prevent the above; depending on what their purposes is they could perhaps pass a weak_ptr instead. The advatage of passing shared_ptr& instead is that it is faster code, since the pointer doesn't have to lock a mutex and increment variables. - Some methods take in a reference instead of a pointer even if they modify the parameter. Generally, the convention is to pass in a pointer in this case; Google's style guide(https://google.github. io/styleguide/cppguide.html#Reference_Arguments <https://google.github.io/styleguide/cppguide.html#Reference_Arguments>) states this as well. So I would recommend that we do this as well. Other notes: - Some of the publicly-visible APIs did change. So a major version bump would be appropriate. - The indent style is rather inconsistent to say the least. I made an astyle config for it and applied it to all files; it's a separate commit since it touches everything. - I want to run the code through cppcheck to help find bugs. - In addition to cppcheck, adding warnings for GCC at least can help find possible bugs in the code. Flags that the Linux kernel uses(note C not C++, but most of these are still applicable). This is not an exhaustive list and is a subset of -Wall, since -Wall can produce a lot of output that is not always helpful. This is from the main Makefile. KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ -fno-strict-aliasing -fno-common \ -Werror-implicit-function-declaration \ -Wno-format-security \ -std=gnu8 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,) KBUILD_CFLAGS += $(call cc-disable-warning,maybe-uninitialized,) KBUILD_CFLAGS += $(call cc-option,-Werror=strict-prototypes) # Prohibit date/time macros, which would make the build non-deterministic KBUILD_CFLAGS += $(call cc-option,-Werror=date-time) # enforce correct pointer usage KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types) If anybody wants to try it out, the commit that it works on(without style changes) is this one(I accidentally committed binaries, so this commit simply removes them): https://github.com/rm5248/log4cxx-testing/commit/10f8fdd99bde2410ee92fefb46a414a5bf12669a Otherwise, the commit with all of the formatting changes is here: https://github.com/rm5248/log4cxx-testing/commit/6c5e3f3ba7075fbe7d82001e90510e4ed9c19029 -Robert Middleton On Sun, Nov 20, 2016 at 12:10 PM, Robert Middleton <osfan6...@gmail.com> wrote: > > > On Sun, Nov 20, 2016 at 9:39 AM, Thorsten Schöning <tschoen...@am-soft.de> > wrote: > >> Guten Tag Robert Middleton, >> am Sonntag, 20. November 2016 um 03:31 schrieben Sie: >> >> > 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 >> >> Great work. Any tips on how I could easily merge that with a separate >> SVN branch? Especially if you keep committing. I guess I'll try >> something using the SVN bridge of GitHub... >> > > It depends on how you want to merge it in. If you don't care about the > history, just do a 'git diff' to whatever the first commit was, which will > essentially squash everything into one diff that you can apply via patch. > > >> >> > This uses a quick and dirty mechanism in autotools to switch >> > between C++11 and boost.[...] >> >> You support autotools and one can easily provide the macros >> independently, doesn't look that "dirty" to me. >> > > The only thing that it doesn't do is run checks to make sure that either > the compiler supports C++11, or if boost is installed; that's the dirty > part of it. Ideally it should check to see what is installed and fail if > nothing is found in the configure phase. > > >> >> > 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. >> >> My understanding of the codebase is that it was converted mainly >> directly from Java and you have instanceof there. >> >> Mit freundlichen Grüßen, >> >> Thorsten Schöning >> >> -- >> Thorsten Schöning E-Mail: thorsten.schoen...@am-soft.de >> AM-SoFT IT-Systeme http://www.AM-SoFT.de/ >> >> Telefon...........05151- 9468- 55 >> Fax...............05151- 9468- 88 >> Mobil..............0178-8 9468- 04 >> >> AM-SoFT GmbH IT-Systeme, Brandenburger Str. 7c, 31789 Hameln >> AG Hannover HRB 207 694 - Geschäftsführer: Andreas Muchow >> >> >
