Closed cbs228 closed 1 month ago
As I wrote elsewhere, until some of the existing known issues are resolved, I'm not interested in spending a lot of my time fixing things that don't seem to be broken. That said, if you want to overhaul the logging system, you are welcome create a pull request to implement the necessary changes. I am OK with using a logging implementation added as a new item in the lib directory or with organic changes to the existing implementation. Rather than trying to evaluate the alternatives you propose, I suggest that you choose the approach/library that you prefer. You should confirm that what you choose to do is compatible with the existing usage. The following are some factors you should probably consider.
If you have additional questions, ask them here. I don't intend to do a detailed code review for a pull request addressing this issue. Instead, I will compile, run any related tests you choose to create, and then run ardopcf to compare log output with results produced prior to the changes. I'll do this on each of the supported build platforms.
This was done by PR #88, which has been merged into the develop branch. The changes will be merged into the master branch when the next release of ardopcf is produced.
(Continued from discussion in #50)
The existing console and file logging framework has some issues:
Mismatched
printf()
format strings and arguments. Functions likeDebugprintf()
did not take advantage of GCC's__printflike
attribute to check format codes. Some invocations had missing arguments that would lead to UB and probably crashes.Use of unsafe unbounded
sprintf()
andstrcat()
functions. These need to be replaced.The logger should be split into its own header and object file. Right now:
Symbols are declared in
ardopcommon.h
, which has a lot of other unrelated stuff.The implementation is part of the platform-specific code. This may introduce unwanted platform-specific differences.
Global variables are mixed with other, unrelated symbols.
If you don't mind the logs looking a bit different, it may be more expeditious and safer to replace the logger with something like zf_log or rxi/log.c. Of these, zf_log looks much more mature. Both are pure C, require no build system, and are MIT-licensed. There are undoubtedly many others.