I'm not sure what the status is of log4cxx at the moment, but progress has been made on my branch since the last time I posted. I have fixed all of the problems that the unit tests found once I converted everything over to smart pointers. Fortunately, there are a large number of unit tests that cover all aspects of the code, so the large majority of issues should have been fixed as of this point. This still requires testing on actual user programs though.
git link: https://github.com/rm5248/log4cxx-testing/tree/smart_pointers -Robert Middleton On Wed, Nov 23, 2016 at 6:43 PM, Robert Middleton <osfan6...@gmail.com> wrote: > 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 >>> >>> >> >
