Reducing the final binary size

[obones]

While working with rtl_43 targeting the ESP32, I must take into account the fact that there is very limited space on such a device.
This is why I have made pull requests in the past for various improvements in such scenarios, most of which have been merged, save for the most complex one manchester_bitrow still being discussed in issue #1726

Today, I'd like to talk about another solution that I have put in place, but before submitting a PR, I believe there are some points that need to be clarified.
The base idea is revolving around the fact that the linker removes unused elements from the final binary image, for instance, unused methods, unused constants and so on...

In the current sources, there are quite a few calls to log functions such as decoder_logf(), FATAL() or print_log() and while they are kept silent by the log level, the argument values they are given are still in the final binary image.

On the ESP32 target, it is quite acceptable to have those methods entirely removed because it is not a target that can be used for rtl_433 own development and so debug messages are of little use.

In order to achieve this, I have done changes like this:

#ifndef RTL_433_DISABLE_FATAL_LOG
#define WARN(what) fprintf(stderr, "WARNING: " what " from " FILE_LINE "\n")
#else
#define WARN(what) /**/
#endif

#ifndef RTL_433_DISABLE_DECODER_LOG
void decoder_log(r_device *decoder, int level, char const *func, char const *msg);
void decoder_logf(r_device *decoder, int level, char const *func, _Printf_format_string_ const char *format, ...)
#if defined(__GNUC__) || defined(__clang__)
        __attribute__((format(printf, 4, 5)))
#endif
        ;
#else
// use "empty macros" so that the unused arguments given hard coded strings will not appear in the final binary.
// Note that the decoder_logf macro uses its variadic arguments to avoid emitting warnings about unused variables in some decoders.
// Calling printf with an empty format string is a not really a noop, and it also emits a warning so we use a blackhole function 
// designed just for this.
#define decoder_log(decoder, level, func, msg) /**/
#define decoder_logf(decoder, level, func, format, ...) decoder_logf_blackhole(0, ## __VA_ARGS__)

void decoder_logf_blackhole(uint8_t dummy, ...);
#endif

Basically, I define RTL_433_DISABLE_DECODER_LOG and RTL_433_DISABLE_FATAL_LOG symbols for my target and I see a decrease in final binary size going from this:

Flash: [========  ]  78.3% (used 1487966 bytes from 1900544 bytes)

to this:

Flash: [========  ]  76.7% (used 1458070 bytes from 1900544 bytes) 

29896 bytes might not seem much on a Linux or Windows platform, but in my case it is quite significant. As those tests were done a few weeks ago, without having modified the logger.h file, I believe the savings would be a bit greater.

My initial idea was to leave the original method declarations at the top of the include file so that they are the first visible when opening the file. But the more I look at it, the more I dislike the fact that its a negative switch with an else part:

#ifndef DISABLE_FEATURE
#else
#endif

To me, it makes it difficult to remember that the else part is for when the feature is disabled, especially when there more than a few lines in each section.
What do you think? Shouldn't I use the more natural if then else construction?

Second, there is the "blackhole" function that is there because some plugins prepare some values that are only used as variadic arguments to the f versions. Initially, I did not use the variadic arguments, but this lead to warnings inside those plugins code because a variable was assigned a value that is never used. My goal being to be the least intrusive as possible, I introduced the blackhole method that is there with an empty body, thus keeping the compiler happy, while allowing to have the constant format string removed from the final binary.
Do you agree with that approach?

Thanks for your replies.

[zuckschwerdt]

Aside: One thing to quickly note/explain, because it's unusual and not really visible: the "log levels" are not meant to be fixed filters (at compile time or startup) but each output should be able to select some maximum level (even dynamically). E.g. the HTTP-API would likely always consume all levels and offer the messages when requested. This in turn means that usually logging can't be optimized away.
There is then also the concept of verbosity to silence most messages before being emitted. I.e. verbosity filtering at the source and log level filtering at the destination.

[obones]

Ah, nice explanation here, I did not notice such possibilities in my own usage.

That being said, the ESP32 project I'm working on mostly uses the devices and does its own output according to its own rules. So the initial proposal still makes sense in this particular use case.

[zuckschwerdt]

Yes, of course. I like the idea of switching off all (optional) logging. I still have to think through possible options to do that.