drakvuf
drakvuf copied to clipboard
Adoping a C++ style guide
Hi, during its development, DRAKVUF had quite a of of different people working on it often with varying opinions, C++ knowledge and stylistic preferences. This led to inconsistencies between some parts of the code and will always be a a source of disagreement between developers.
My proposal is to adopt Google C++ Style Guide to ensure that with time the codebase will become more uniform and thus easier to understand and develop.
This type of standardization is great if we can get contributors to agree. But as a principle I prefer not to enforce it for plugins as I believe people should be allowed to write plugins in any way they feel like. The basic plugin interface and libdrakvuf
API will be kept as minimalistic as possible from a C++ perspective for this reason.
More complex C++ layers like plugin_ex
and libusermode
are strictly opt-in for plugins to use. I largely defer style discussion and C++ feature usage on these to whoever uses them and puts the effort into maintaining it/reviewing changes. If there is a critical disagreement between groups its always possible to fork them and just let everyone use their own preferred wrapper with their preferred style.
So I would rather reframe this standardization topic for these subsystems only. I want to let you guys figure out what standard you want to use for it and enforce going forward. I think it would be valuable and reduce friction in the long term to agree on a subset of C++ features to use. So let's hear from others what they think about it.
/cc @disaykin @skvl
Any auto-mantained code style is fine for me.
I second that, if we can add a style check to Github Actions for this that would be fantastic.
Although I don't contribute lately I share the point of view from @tklengyel
https://github.com/cpplint/cpplint looks like a solid tool for the job
# cpplint --filter=-whitespace,-legal,-build/header_guard src/libusermode/*
Ignoring src/libusermode/Makefile; not a valid file name (h++, cc, hh, cxx, cuh, hxx, c++, hpp, cu, c, h, cpp)
Done processing src/libusermode/Makefile
Ignoring src/libusermode/Makefile.am; not a valid file name (h++, cc, hh, cxx, cuh, hxx, c++, hpp, cu, c, h, cpp)
Done processing src/libusermode/Makefile.am
Ignoring src/libusermode/Makefile.in; not a valid file name (h++, cc, hh, cxx, cuh, hxx, c++, hpp, cu, c, h, cpp)
Done processing src/libusermode/Makefile.in
Skipping input 'src/libusermode/printers': Can't open for reading
src/libusermode/running.cpp:112: Found C system header after C++ system header. Should be: running.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/running.cpp:113: Found C system header after C++ system header. Should be: running.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/running.cpp:114: Found C system header after C++ system header. Should be: running.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/running.cpp:115: Found C system header after C++ system header. Should be: running.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/running.cpp:116: Found C system header after C++ system header. Should be: running.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/running.cpp:117: Found C system header after C++ system header. Should be: running.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/running.cpp:118: Found C system header after C++ system header. Should be: running.h, c system, c++ system, other. [build/include_order] [4]
Done processing src/libusermode/running.cpp
src/libusermode/uh-private.hpp:111: Found C system header after C++ system header. Should be: uh-private.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/uh-private.hpp:169: Using C-style cast. Use reinterpret_cast<rh_data_t*>(...) instead [readability/casting] [4]
src/libusermode/uh-private.hpp:234: Single-parameter constructors should be marked explicit. [runtime/explicit] [5]
src/libusermode/uh-private.hpp:215: Add #include <map> for map<> [build/include_what_you_use] [4]
src/libusermode/uh-private.hpp:226: Add #include <string> for string [build/include_what_you_use] [4]
Done processing src/libusermode/uh-private.hpp
src/libusermode/userhook.cpp:123: Found C system header after C++ system header. Should be: userhook.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/userhook.cpp:124: Found C system header after C++ system header. Should be: userhook.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/userhook.cpp:125: Found C system header after C++ system header. Should be: userhook.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/userhook.cpp:126: Found C system header after C++ system header. Should be: userhook.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/userhook.cpp:127: Found C system header after C++ system header. Should be: userhook.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/userhook.cpp:128: Found C system header after C++ system header. Should be: userhook.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/userhook.cpp:129: Found C system header after C++ system header. Should be: userhook.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/userhook.cpp:185: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
src/libusermode/userhook.cpp:214: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
src/libusermode/userhook.cpp:317: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
src/libusermode/userhook.cpp:326: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
src/libusermode/userhook.cpp:331: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
src/libusermode/userhook.cpp:403: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
src/libusermode/userhook.cpp:404: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
src/libusermode/userhook.cpp:717: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
src/libusermode/userhook.cpp:718: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
src/libusermode/userhook.cpp:865: Add #include <utility> for move [build/include_what_you_use] [4]
Done processing src/libusermode/userhook.cpp
src/libusermode/userhook.hpp:111: Found C system header after C++ system header. Should be: userhook.h, c system, c++ system, other. [build/include_order] [4]
src/libusermode/userhook.hpp:166: Add #include <utility> for move [build/include_what_you_use] [4]
src/libusermode/userhook.hpp:273: Add #include <string> for string [build/include_what_you_use] [4]
Done processing src/libusermode/userhook.hpp
Total errors found: 32