From a2017f68f16541a2dfcae17b151ba6feea0994fc Mon Sep 17 00:00:00 2001 From: Teddy Reed Date: Mon, 15 Aug 2016 01:33:17 -0700 Subject: [PATCH] Add clang-format rules from 3.6 (#2360) --- .clang-format | 50 ++++++++++++----- CMakeLists.txt | 2 +- Doxyfile | 6 +- Makefile | 25 ++++++--- docs/wiki/development/building.md | 28 ++++++++++ osquery/dispatcher/scheduler.cpp | 8 ++- osquery/remote/transports/tls.h | 17 +++--- osquery/sql/sql.cpp | 26 ++++++--- osquery/sql/sqlite_math.cpp | 58 +++++++++++++------- osquery/tables/system/darwin/kernel_info.cpp | 3 + tools/audit.sh | 54 ++++++++++++++++++ tools/lib.sh | 9 ++- tools/provision.sh | 1 + 13 files changed, 221 insertions(+), 66 deletions(-) create mode 100755 tools/audit.sh diff --git a/.clang-format b/.clang-format index 398e7f208fe..610734c9d0f 100644 --- a/.clang-format +++ b/.clang-format @@ -1,41 +1,65 @@ --- +Language: Cpp +# BasedOnStyle: Google AccessModifierOffset: -1 -ConstructorInitializerIndentWidth: 4 -AlignEscapedNewlinesLeft: true -AlignTrailingComments: false +AlignAfterOpenBracket: true +AlignEscapedNewlinesLeft: false +AlignOperands: true +AlignTrailingComments: false # differs +AllowShortBlocksOnASingleLine: false AllowAllParametersOfDeclarationOnNextLine: true -AllowShortIfStatementsOnASingleLine: false -AllowShortLoopsOnASingleLine: false +AllowShortCaseLabelsOnASingleLine: false +AllowShortIfStatementsOnASingleLine: false # differs +AllowShortLoopsOnASingleLine: false # differs +AllowShortFunctionsOnASingleLine: Empty +AlwaysBreakAfterDefinitionReturnType: false AlwaysBreakTemplateDeclarations: true AlwaysBreakBeforeMultilineStrings: true -BreakBeforeBinaryOperators: false +BreakBeforeBinaryOperators: false # differs +BreakBeforeTernaryOperators: true BreakConstructorInitializersBeforeComma: false BinPackParameters: false +BinPackArguments: false ColumnLimit: 80 +ConstructorInitializerIndentWidth: 4 ConstructorInitializerAllOnOneLineOrOnePerLine: true +DerivePointerAlignment: false DerivePointerBinding: true -ExperimentalAutoDetectBinPacking: true -IndentCaseLabels: false +IndentCaseLabels: false #differs +IndentWrappedFunctionNames: false +IndentFunctionDeclarationAfterType: false MaxEmptyLinesToKeep: 1 +KeepEmptyLinesAtTheStartOfBlocks: false NamespaceIndentation: None +ObjCBlockIndentWidth: 2 +ObjCSpaceAfterProperty: false ObjCSpaceBeforeProtocolList: false -PenaltyBreakComment: 60 +PenaltyBreakBeforeFirstCallParameter: 1 +PenaltyBreakComment: 300 PenaltyBreakString: 1000 -PenaltyBreakFirstLessLess: 20 +PenaltyBreakFirstLessLess: 120 PenaltyExcessCharacter: 1000000 PenaltyReturnTypeOnItsOwnLine: 200 +PointerAlignment: Left PointerBindsToType: true -SpacesBeforeTrailingComments: 1 +SpacesBeforeTrailingComments: 1 #differs Cpp11BracedListStyle: true Standard: Cpp11 IndentWidth: 2 TabWidth: 8 UseTab: Never BreakBeforeBraces: Attach -IndentFunctionDeclarationAfterType: false SpacesInParentheses: false +SpacesInSquareBrackets: false +SpacesInAngles: false SpaceInEmptyParentheses: false SpacesInCStyleCastParentheses: false -SpaceAfterControlStatementKeyword: true +SpaceAfterCStyleCast: false +SpacesInContainerLiterals: true SpaceBeforeAssignmentOperators: true +SpaceAfterControlStatementKeyword: true +ContinuationIndentWidth: 4 +CommentPragmas: '^ IWYU pragma:' +ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH ] +SpaceBeforeParens: ControlStatements ... diff --git a/CMakeLists.txt b/CMakeLists.txt index a379819f44a..9ed83389d5e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -507,7 +507,7 @@ add_custom_target( # make format add_custom_target( format - "${PYTHON_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/tools/formatting/git-clang-format.py" + "${PYTHON_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/tools/formatting/git-clang-format.py" "-f" WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" COMMENT "Formatting code staged code changes with clang-format" VERBATIM ) diff --git a/Doxyfile b/Doxyfile index aede5ab77bc..b9bd0c46889 100644 --- a/Doxyfile +++ b/Doxyfile @@ -991,16 +991,14 @@ VERBATIM_HEADERS = YES # Note: The availability of this option depends on whether or not doxygen was # compiled with the --with-libclang option. # The default value is: NO. - -CLANG_ASSISTED_PARSING = NO +# CLANG_ASSISTED_PARSING = NO # If clang assisted parsing is enabled you can provide the compiler with command # line options that you would normally use when invoking the compiler. Note that # the include paths will already be set by doxygen for the files and directories # specified with INPUT and INCLUDE_PATH. # This tag requires that the tag CLANG_ASSISTED_PARSING is set to YES. - -CLANG_OPTIONS = +# CLANG_OPTIONS = #--------------------------------------------------------------------------- # Configuration options related to the alphabetical class index diff --git a/Makefile b/Makefile index f94edac760a..18e8c86f214 100644 --- a/Makefile +++ b/Makefile @@ -30,10 +30,10 @@ ifneq ($(OSQUERY_DEPS),) else DEPS_DIR = /usr/local/osquery endif -CMAKE := PATH="$(DEPS_DIR)/bin:/usr/local/bin:$(PATH)" \ - CXXFLAGS="-L$(DEPS_DIR)/lib" cmake ../../ -DOCS_CMAKE := PATH="$(DEPS_DIR)/bin:/usr/local/bin:$(PATH)" \ - CXXFLAGS="-L$(DEPS_DIR)/lib" cmake ../ +PATH_SET := PATH="$(DEPS_DIR)/bin:/usr/local/bin:$(PATH)" +CMAKE := $(PATH_SET) CXXFLAGS="-L$(DEPS_DIR)/lib" cmake ../../ +DOCS_CMAKE := $(PATH_SET) CXXFLAGS="-L$(DEPS_DIR)/lib" cmake ../ +FORMAT_COMMAND := python tools/formatting/git-clang-format.py "--commit" "master" "-f" DEFINES := CTEST_OUTPUT_ON_FAILURE=1 .PHONY: docs build @@ -46,6 +46,9 @@ docs: .setup @cd build && DOCS=True $(DOCS_CMAKE) && \ $(DEFINES) $(MAKE) docs --no-print-directory $(MAKEFLAGS) +format_master: + @$(PATH_SET) $(FORMAT_COMMAND) + debug: .setup @cd build/debug_$(BUILD_DIR) && DEBUG=True $(CMAKE) && \ $(DEFINES) $(MAKE) --no-print-directory $(MAKEFLAGS) @@ -78,9 +81,13 @@ test_debug_sdk: .setup @cd build/debug_$(BUILD_DIR) && SDK=True DEBUG=True $(CMAKE) && \ $(DEFINES) $(MAKE) test --no-print-directory $(MAKEFLAGS) -build: - cd build/$(BUILD_DIR) && \ - $(DEFINES) $(MAKE) --no-print-directory $(MAKEFLAGS) +check: + @$(PATH_SET) cppcheck --quiet --enable=all --error-exitcode=0 \ + -I ./include ./osquery + # We want check to produce an error if there are critical issues. + @echo "" + @$(PATH_SET) cppcheck --quiet --enable=warning --error-exitcode=1 \ + -I ./include ./osquery debug_build: cd build/debug_$(BUILD_DIR) && \ @@ -141,7 +148,7 @@ endif package: .setup # Alias for packages (do not use CPack) @cd build/$(BUILD_DIR) && PACKAGE=True $(CMAKE) && \ - $(DEFINES) $(MAKE) packages --no-print-directory $(MAKEFLAGS) + $(DEFINES) $(MAKE) packages/fast --no-print-directory $(MAKEFLAGS) debug_package: .setup @cd build/debug_$(BUILD_DIR) && DEBUG=True PACKAGE=True $(CMAKE) && \ @@ -149,7 +156,7 @@ debug_package: .setup packages: .setup @cd build/$(BUILD_DIR) && PACKAGE=True $(CMAKE) && \ - $(DEFINES) $(MAKE) packages --no-print-directory $(MAKEFLAGS) + $(DEFINES) $(MAKE) packages/fast --no-print-directory $(MAKEFLAGS) debug_packages: @cd build/debug_$(BUILD_DIR) && DEBUG=True PACKAGE=True $(CMAKE) && \ diff --git a/docs/wiki/development/building.md b/docs/wiki/development/building.md index 6f19a817fd0..3c28c287f4f 100644 --- a/docs/wiki/development/building.md +++ b/docs/wiki/development/building.md @@ -71,6 +71,34 @@ The binaries are built to a distro-specific folder within *build* and symlinked $ ls -la ./build/linux/osquery/ ``` +## Submitting Pull Requests + +Once you have made changes you'll want to submit them to Github as a Pull Request. There are tons of wonderful guides and documentation around Pull Requests, and that is just out of scope for this wiki-- but consider the following workflow: + +``` +$ git checkout -b new_feature1 +$ # write some code! +$ make -j 4 +$ git commit -m "New Feature: do something wonderful" +$ git push +``` + +This assumes your remote `origin` is your osquery fork, and that you receive updates from an `upstream`. It is also common to use `origin` as `facebook/osquery` then add your fork as a target named after your Github username. + +In that case the final push becomes `git push USERNAME`. + +### Testing changes + +Our Jenkins CI will test your changes in three steps. + +1. A code audit is run using `./tools/audit.sh`. +2. The code is rebuilt, built again for release, then a package is generated using `./tools/build.sh` on various Linux and OS X versions. +3. The same step is run on Windows 10. + +The audit step attempts to build the documentation, run code formatting checks, and a brief static code analysis. The formatting check is performed with `clang-format` (installed with `make deps` to your osquery dependencies directory). Your changes are compared against the local `master` branch. Within the build host this is always the TIP of `facebook/osquery`, but locally the branch may be behind. + +To speed up the format auditing process please configure your code editor to run `clang-format` on files changed. Or periodically during your development run `make format`. Running `make check` is also helpful, but it will use `cppcheck` which is not installed by default. + ## Dependencies and build internals The `make deps` command is fairly intense and serves two purposes: (1) to communicate a standard set of environment setup instructions for our build and test nodes, (2) to provide an environment for reproducing errors. The are wonderful auxiliary benefits such as controlling the compiler and compile flags for almost all of our dependencies, controlling security-related features for dependencies, allowing a "mostly" universal build for Linux that makes deployment simple. To read more about the motivation and FAQ for our dependencies environment see the [Github Refererence #2253](https://github.com/facebook/osquery/issues/2253). diff --git a/osquery/dispatcher/scheduler.cpp b/osquery/dispatcher/scheduler.cpp index b772d52da38..5a4c2f1d24b 100644 --- a/osquery/dispatcher/scheduler.cpp +++ b/osquery/dispatcher/scheduler.cpp @@ -50,8 +50,8 @@ inline SQL monitor(const std::string& name, const ScheduledQuery& query) { } } // Always called while processes table is working. - Config::getInstance().recordQueryPerformance(name, t1 - t0, size, r0[0], - r1[0]); + Config::getInstance().recordQueryPerformance( + name, t1 - t0, size, r0[0], r1[0]); } return sql; } @@ -148,7 +148,9 @@ void SchedulerRunner::start() { } } -void startScheduler() { startScheduler(FLAGS_schedule_timeout, 1); } +void startScheduler() { + startScheduler(FLAGS_schedule_timeout, 1); +} void startScheduler(unsigned long int timeout, size_t interval) { Dispatcher::addService(std::make_shared(timeout, interval)); diff --git a/osquery/remote/transports/tls.h b/osquery/remote/transports/tls.h index c176daa813c..c5073627c35 100644 --- a/osquery/remote/transports/tls.h +++ b/osquery/remote/transports/tls.h @@ -38,12 +38,12 @@ void ERR_remove_state(unsigned long); // On OS X these symbols are marked deprecated and clang will warn against // us including them. We are squashing the noise for OS X's OpenSSL only. // clang-format off -_Pragma("clang diagnostic push") -_Pragma("clang diagnostic ignored \"-Wdeprecated-declarations\"") -_Pragma("clang diagnostic ignored \"-Wunused-local-typedef\"") -_Pragma("clang diagnostic ignored \"-W#pragma-messages\"") +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +#pragma clang diagnostic ignored "-Wunused-local-typedef" +#pragma clang diagnostic ignored "-W#pragma-messages" #include -_Pragma("clang diagnostic pop") +#pragma clang diagnostic pop // clang-format on #include @@ -55,7 +55,8 @@ namespace osquery { /// Path to optional TLS client secret key, used for enrollment/requests. DECLARE_string(tls_client_key); -/// Path to optional TLS client certificate (PEM), used for enrollment/requests. +/// Path to optional TLS client certificate (PEM), used for +/// enrollment/requests. DECLARE_string(tls_client_cert); /// TLS server hostname. @@ -106,7 +107,9 @@ class TLSTransport : public Transport { private: /// Testing-only, disable peer verification. - void disableVerifyPeer() { verify_peer_ = false; } + void disableVerifyPeer() { + verify_peer_ = false; + } /// Set TLS-client authentication options. void setClientCertificate(const std::string& certificate_file, diff --git a/osquery/sql/sql.cpp b/osquery/sql/sql.cpp index 1493a430489..1fb55246912 100644 --- a/osquery/sql/sql.cpp +++ b/osquery/sql/sql.cpp @@ -20,22 +20,34 @@ namespace osquery { FLAG(int32, value_max, 512, "Maximum returned row value size"); -SQL::SQL(const std::string& q) { status_ = query(q, results_); } +SQL::SQL(const std::string& q) { + status_ = query(q, results_); +} -const QueryData& SQL::rows() const { return results_; } +const QueryData& SQL::rows() const { + return results_; +} -bool SQL::ok() { return status_.ok(); } +bool SQL::ok() { + return status_.ok(); +} -const Status& SQL::getStatus() const { return status_; } +const Status& SQL::getStatus() const { + return status_; +} -std::string SQL::getMessageString() { return status_.toString(); } +std::string SQL::getMessageString() { + return status_.toString(); +} static inline void escapeNonPrintableBytes(std::string& data) { std::string escaped; + // clang-format off char const hex_chars[16] = { - '0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'A', 'B', 'C', 'D', 'E', 'F', + '0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'A', 'B', 'C', 'D', 'E', 'F', }; + // clang-format on bool needs_replacement = false; for (size_t i = 0; i < data.length(); i++) { diff --git a/osquery/sql/sqlite_math.cpp b/osquery/sql/sqlite_math.cpp index 603009432d2..14a106871a4 100644 --- a/osquery/sql/sqlite_math.cpp +++ b/osquery/sql/sqlite_math.cpp @@ -27,10 +27,12 @@ namespace osquery { using DoubleDoubleFunction = std::function; -// force use of the double(double) math functions -// without these lambda functions, MSVC will error -// because it fails to select an overload compatible with -// DoubleDoubleFunction +/** + * Force use of the double(double) math functions without these lambda + * functions, MSVC will error because it fails to select an overload compatible + * with DoubleDoubleFunction. + */ +// clang-format off #define SIN_FUNC [](double a)->double { return sin(a); } #define COS_FUNC [](double a)->double { return cos(a); } #define TAN_FUNC [](double a)->double { return tanl(a); } @@ -43,7 +45,7 @@ using DoubleDoubleFunction = std::function; #define EXP_FUNC [](double a)->double { return expl(a); } #define CEIL_FUNC [](double a)->double { return ceil(a); } #define FLOOR_FUNC [](double a)->double { return floor(a); } - +// clang-format on /** * @brief Call a math function that takes a double and returns a double. @@ -95,7 +97,9 @@ static void atanFunc(sqlite3_context *context, int argc, sqlite3_value **argv) { callDoubleFunc(context, argc, argv, ATAN_FUNC); } -static double cot(double x) { return 1.0 / tan(x); } +static double cot(double x) { + return 1.0 / tan(x); +} static void cotFunc(sqlite3_context *context, int argc, sqlite3_value **argv) { callDoubleFunc(context, argc, argv, cot); @@ -177,10 +181,14 @@ static void floorFunc(sqlite3_context *context, } /** Convert Degrees into Radians */ -static double deg2rad(double x) { return x * M_PI / 180.0; } +static double deg2rad(double x) { + return x * M_PI / 180.0; +} /** Convert Radians into Degrees */ -static double rad2deg(double x) { return 180.0 * x / M_PI; } +static double rad2deg(double x) { + return 180.0 * x / M_PI; +} static void rad2degFunc(sqlite3_context *context, int argc, @@ -209,22 +217,34 @@ void registerMathExtensions(sqlite3 *db) { // somewhat deprecated/legacy work by Liam Healy from 2010 in the extension // functions contribution. static const struct FuncDef aFuncs[] = { - {"sqrt", 1, sqrtFunc}, {"acos", 1, acosFunc}, - {"asin", 1, asinFunc}, {"atan", 1, atanFunc}, - {"cos", 1, cosFunc}, {"sin", 1, sinFunc}, - {"tan", 1, tanFunc}, {"cot", 1, cotFunc}, - {"exp", 1, expFunc}, {"log", 1, logFunc}, - {"log10", 1, log10Func}, {"power", 2, powerFunc}, - {"ceil", 1, ceilFunc}, {"floor", 1, floorFunc}, - {"degrees", 1, rad2degFunc}, {"radians", 1, deg2radFunc}, + {"sqrt", 1, sqrtFunc}, + {"acos", 1, acosFunc}, + {"asin", 1, asinFunc}, + {"atan", 1, atanFunc}, + {"cos", 1, cosFunc}, + {"sin", 1, sinFunc}, + {"tan", 1, tanFunc}, + {"cot", 1, cotFunc}, + {"exp", 1, expFunc}, + {"log", 1, logFunc}, + {"log10", 1, log10Func}, + {"power", 2, powerFunc}, + {"ceil", 1, ceilFunc}, + {"floor", 1, floorFunc}, + {"degrees", 1, rad2degFunc}, + {"radians", 1, deg2radFunc}, {"pi", 0, piFunc}, }; for (size_t i = 0; i < sizeof(aFuncs) / sizeof(struct FuncDef); i++) { - sqlite3_create_function(db, aFuncs[i].zFunctionName, aFuncs[i].nArg, - SQLITE_UTF8, nullptr, aFuncs[i].xFunc, nullptr, + sqlite3_create_function(db, + aFuncs[i].zFunctionName, + aFuncs[i].nArg, + SQLITE_UTF8, + nullptr, + aFuncs[i].xFunc, + nullptr, nullptr); } } } - diff --git a/osquery/tables/system/darwin/kernel_info.cpp b/osquery/tables/system/darwin/kernel_info.cpp index fea99067dfd..160e57db9fc 100644 --- a/osquery/tables/system/darwin/kernel_info.cpp +++ b/osquery/tables/system/darwin/kernel_info.cpp @@ -58,6 +58,7 @@ std::string getCanonicalEfiDevicePath(const CFDataRef& data) { } else if (node->SubType == MEDIA_HARDDRIVE_DP) { // Extract the device UUID to later join with block devices. auto uuid = ((const HARDDRIVE_DEVICE_PATH*)node)->Signature; + // clang-format off boost::uuids::uuid hdd_signature = {{ uuid[3], uuid[2], uuid[1], uuid[0], uuid[5], uuid[4], @@ -65,6 +66,8 @@ std::string getCanonicalEfiDevicePath(const CFDataRef& data) { uuid[8], uuid[9], uuid[10], uuid[11], uuid[12], uuid[13], uuid[14], uuid[15], }}; + // clang-format on + path += boost::to_upper_copy(boost::uuids::to_string(hdd_signature)); } } diff --git a/tools/audit.sh b/tools/audit.sh new file mode 100755 index 00000000000..f51762a14bc --- /dev/null +++ b/tools/audit.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash + +# Copyright (c) 2014-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +set -e + +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +source $SCRIPT_DIR/lib.sh + +function check_format() { + # Create a master branch if it does not exist. + if ! git rev-parse --verify master &> /dev/null; then + git fetch origin master &> /dev/null + git branch master FETCH_HEAD &> /dev/null || true + fi + + # Format and show the status + make format_master + + if [[ `git diff --name-only | wc -l | awk '{print $1}'` = "0" ]]; then + return 0 + else + git status + return 1 + fi +} + +function audit() { + log "Running various code/change audits!" + log "Initializing and updating all submodules" + checkout_thirdparty + + # Check the docs creation + echo "" + log "Running: make docs" + make docs + + echo "" + log "Running: make check" + make check + + echo "" + log "Running: make format" + check_format +} + +audit + +exit 0 diff --git a/tools/lib.sh b/tools/lib.sh index 776d6a21be8..3e8bc38335e 100644 --- a/tools/lib.sh +++ b/tools/lib.sh @@ -100,13 +100,16 @@ function build_kernel_cleanup() { $MAKE kernel-test-unload || sudo reboot } -function initialize() { - DISTRO=$1 - +function checkout_thirdparty() { # Reset any work or artifacts from build tests in TP. (cd third-party && git reset --hard HEAD) git submodule init git submodule update +} + +function initialize() { + DISTRO=$1 + checkout_thirdparty # Remove any previously-cached variables rm build/$DISTRO/CMakeCache.txt >/dev/null 2>&1 || true diff --git a/tools/provision.sh b/tools/provision.sh index 3287f28ea05..6cdb64a267c 100755 --- a/tools/provision.sh +++ b/tools/provision.sh @@ -252,6 +252,7 @@ function platform_darwin_main() { brew_tool readline brew_tool sqlite brew_tool makedepend + brew_tool clang-format local_brew_dependency openssl --without-test local_brew_link openssl