“tinyformat: Too many conversion specifiers in format string” #9423

1. 
   rebroad commented at 4:33 pm on December 25, 2016: contributor

   I don’t think I’ve seen an issue raised for this - it’s more of a development issue than a runtime issue but if encounter it often, which is that if there is an incorrect number of arguments in a LogPrint() or LogPrintf() then no compile error/warning is given, it will cause the code to SIGABRT, and a gdb backtrace never provides much useful information about where the error in the code is. Only the name of the thread is given which can often leave a lot of searching or git bisecting. This common problem could be made easier to debug…
2. fanquake added the label Utils and libraries on Dec 26, 2016
3. 
   gmaxwell commented at 9:22 am on December 28, 2016: contributor

   It’s basically impossible to give a compile error for this unfortunately. It’s only possible for printf because the compiler basically implements the whole function internally.
4. 
   ryanofsky commented at 2:01 pm on December 28, 2016: member

   Could __attribute__ format work?

   http://clang.llvm.org/docs/AttributeReference.html#format-gnu-format https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

5. 
   laanwj commented at 8:14 am on January 2, 2017: member

   it will cause the code to SIGABRT, and a gdb backtrace never provides much useful information about where the error in the code is

   That’s not entirely correct. It is set up to raise an exception.

   0 #define TINYFORMAT_ERROR(reasonString) throw std::runtime_error(reasonString)
   

   In the case of logging it would be OK to just log a warning message instead of raising an exception. The various Log* could catch this exception and do that.

   strprintf OTOH should not, because it is also used in more critical places than logging (such as formatting RPC output), and a programmer error there to specify not enough conversion specifiers is a critical problem.

6. 
   laanwj commented at 8:22 am on January 2, 2017: member

   An external script would also be able to check this, without too much trouble as the format strings must be static. That the script that converts translations checks the number of formatting %’s for exactly this reason, so that translation don’t introduce crashes.

   In the long run I’d prefer to abandon strprintf-ish syntax and use, at least for logging, the more C++ish way of

   0Logger(Error,"rpc") << "HTTP: starting" << rpcThreads << "worker threads";
   

   This is what tinyformat does internally (and would be similar to qDebug. It is a type-safe compatibility layer from printf-like vararg syntax to c++ formatting.

   Using tinyformat was a workaround to make it possible to keep most of the logging lines the same while still avoiding libc printf’s pitfalls such as variability between platforms and type-safety issues. Lack of compile-time checking of number of arguments is annoying though.

7. laanwj referenced this in commit 9f51fe897d on Mar 9, 2017
8. laanwj referenced this in commit 3b092bd9b6 on Mar 12, 2017
9. practicalswift referenced this in commit c3806ef17a on Mar 13, 2017
10. practicalswift referenced this in commit c049186a87 on Apr 27, 2017
11. 
    fanquake commented at 9:07 am on October 7, 2017: member

    Fixed in #9423.
12. fanquake closed this on Oct 7, 2017

    ---

13. HashUnlimited referenced this in commit 2c399a85fc on Feb 28, 2018
14. rebroad referenced this in commit 36fddcee78 on Feb 19, 2021
15. rebroad referenced this in commit 345d9ccaad on Mar 28, 2021
16. MarcoFalke locked this on Sep 8, 2021

Contributors
rebroad gmaxwell ryanofsky laanwj fanquake

Labels
Utils/log/libs