Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cstdio name collision solution #74

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

diggit
Copy link

@diggit diggit commented Feb 27, 2020

I had issues with name collision when including cstdio #66. Solution with mapping to wrap functions is taken from bisqwit/tinyprintf. What do you think about it?

Consider this a quick POC rather than a final code solution.

@codecov-io
Copy link

codecov-io commented Feb 27, 2020

Codecov Report

Merging #74 (92e71f3) into master (d3b9846) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #74   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          359       359           
=========================================
  Hits           359       359           
Impacted Files Coverage Δ
printf.c 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3b9846...66e17f1. Read the comment docs.

@mpaland
Copy link
Owner

mpaland commented Feb 27, 2020

Hi Patrik,
what exactly is the problem with the trailing underscore?
Is it defined that way in your cstudio?

@diggit
Copy link
Author

diggit commented Feb 28, 2020

trailing underscore is not a problem, macros like #define printf printf_ are. When I comment them out, error is gone.

Error:

[1/3] Building CXX object CMakeFiles/TLM.elf.dir/src/main.cpp.obj
FAILED: CMakeFiles/TLM.elf.dir/src/main.cpp.obj 
/usr/bin/arm-none-eabi-g++   -I../src -I../src/printf -I../src/span/include/tcb -I../src/CMSIS/CMSIS/Core/Include -I../src/CMSIS/CMSIS/DSP/Include -I../src/CMSIS/CMSIS/DSP/Source -fno-builtin -Wall -ffunction-sections -fdata-sections -fomit-frame-pointer -mabi=aapcs -std=c++11  -D 'BUILD=385' -D 'GIT_VERSION_SHORT=3256378' -Dtimegm=mktime -DDEBUG=1 -DSTM32F446=1 -DHWVERSION=12 -DTCB_SPAN_NO_EXCEPTIONS -DMPRINTF_DISABLE_DIRECT_MAPPING -Werror=switch -Werror=return-type -Werror=stringop-overflow -Werror=parentheses -Wfatal-errors -Wno-unused-function  -Wall -Wextra -Wundef -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Wnull-dereference -Wcast-align -Wvla -Wmissing-format-attribute -Wuninitialized -Winit-self -Wdouble-promotion -Wstrict-aliasing -Wno-unused-local-typedefs -funwind-tables -Wno-unused-parameter -pedantic -g -Og  -mthumb -mcpu=cortex-m4 -mfloat-abi=hard -mfpu=fpv5-sp-d16 -funsigned-char -funsigned-bitfields -ffunction-sections -fdata-sections -fdiagnostics-color=always -fstack-protector-strong -finline-small-functions -findirect-inlining -Wint-to-pointer-cast -Weffc++ -std=c++2a -Wno-register -fno-rtti -fno-exceptions -fno-threadsafe-statics -MD -MT CMakeFiles/TLM.elf.dir/src/main.cpp.obj -MF CMakeFiles/TLM.elf.dir/src/main.cpp.obj.d -o CMakeFiles/TLM.elf.dir/src/main.cpp.obj -c ../src/main.cpp
In file included from ../src/main.cpp:19:
/usr/arm-none-eabi/include/c++/9.2.0/cstdio:137:11: error: '::sprintf' has not been declared
  137 |   using ::sprintf;
      |           ^~~~~~~
compilation terminated due to -Wfatal-errors.
[2/3] Building C object CMakeFiles/TLM.elf.dir/src/printf/printf.c.obj
ninja: build stopped: subcommand failed.
/*line 18*/ #include "printf/printf.h"
/*line 19*/ #include <cstdio>

If I swap them, error is different:

[1/2] Building CXX object CMakeFiles/TLM.elf.dir/src/main.cpp.obj
FAILED: CMakeFiles/TLM.elf.dir/src/main.cpp.obj 
/usr/bin/arm-none-eabi-g++   -I../src -I../src/printf -I../src/span/include/tcb -I../src/CMSIS/CMSIS/Core/Include -I../src/CMSIS/CMSIS/DSP/Include -I../src/CMSIS/CMSIS/DSP/Source -fno-builtin -Wall -ffunction-sections -fdata-sections -fomit-frame-pointer -mabi=aapcs -std=c++11  -D 'BUILD=385' -D 'GIT_VERSION_SHORT=3256378' -Dtimegm=mktime -DDEBUG=1 -DSTM32F446=1 -DHWVERSION=12 -DTCB_SPAN_NO_EXCEPTIONS -DMPRINTF_DISABLE_DIRECT_MAPPING -Werror=switch -Werror=return-type -Werror=stringop-overflow -Werror=parentheses -Wfatal-errors -Wno-unused-function  -Wall -Wextra -Wundef -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Wnull-dereference -Wcast-align -Wvla -Wmissing-format-attribute -Wuninitialized -Winit-self -Wdouble-promotion -Wstrict-aliasing -Wno-unused-local-typedefs -funwind-tables -Wno-unused-parameter -pedantic -g -Og  -mthumb -mcpu=cortex-m4 -mfloat-abi=hard -mfpu=fpv5-sp-d16 -funsigned-char -funsigned-bitfields -ffunction-sections -fdata-sections -fdiagnostics-color=always -fstack-protector-strong -finline-small-functions -findirect-inlining -Wint-to-pointer-cast -Weffc++ -std=c++2a -Wno-register -fno-rtti -fno-exceptions -fno-threadsafe-statics -MD -MT CMakeFiles/TLM.elf.dir/src/main.cpp.obj -MF CMakeFiles/TLM.elf.dir/src/main.cpp.obj.d -o CMakeFiles/TLM.elf.dir/src/main.cpp.obj -c ../src/main.cpp
In file included from ../src/main.cpp:19:
/usr/arm-none-eabi/include/c++/9.2.0/bits/basic_string.h: In function 'std::string std::__cxx11::to_string(int)':
../src/printf/printf.h:86:19: error: 'vsnprintf_' is not a member of 'std'; did you mean 'isprint'?
   86 | #define vsnprintf vsnprintf_
      |                   ^~~~~~~~~~
compilation terminated due to -Wfatal-errors.
ninja: build stopped: subcommand failed.

@diggit
Copy link
Author

diggit commented Feb 28, 2020

With this wrap workaround, you have to include normal printf headers (<cstdio>) and calls to printf get remapped to __wrap_printf at link time.

@mpaland mpaland self-assigned this Feb 28, 2020
@mpaland
Copy link
Owner

mpaland commented Feb 28, 2020

Uiii, I see, the macro defines create problems. Unusual, but existent.
Going to merge your PS soon.

@diggit
Copy link
Author

diggit commented Feb 29, 2020

I am open to suggestion for improvements, like styling etc.

Also some note about this problem and how to solve it should be put somewhere. Readme or comment in code?

When someone want to use wrapping method in c++, should we suggest to directly include <cstdio> or rather embed inclusion into printf.h via some macro (preferred because in C, you would have to include stdio.h and including one header might be easier)?

default names are mp_printf etc.
to rename to epic_printf, add compiler arg:
`-D'PRINTF_FUNCTION_RENAME\(base\)=epic_\#\#base'`
(example taken from cmake, character escaping might differ)
@diggit
Copy link
Author

diggit commented Dec 23, 2020

A have new way of name collision avoidance. Function names are prefixed by default by mp_ prefix (your nickname shortened). To alter function names, simple macro can be added via compiler flags. Eg -D'PRINTF_FUNCTION_RENAME\(base\)=__wrap_\#\#base' gives all functions __wrap_ prefix (eg. __wrap_printf) to allow link time substitution. Adding -D'PRINTF_FUNCTION_RENAME\(base\)=base' gives functions their native names etc... Tested with cmake. Character escaping may differ in other build systems.
This implementation alters functions names and not just some mapping. Thus making possible that __wrap_ magic.

KarlK90 pushed a commit to qmk/printf that referenced this pull request Jul 7, 2022
…printf.h` prototypes are now `static`, and will not raise any eyebrows when the resulting object is inspected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants