You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kvrocks.apache.org by ti...@apache.org on 2022/10/23 01:32:31 UTC
[incubator-kvrocks] branch unstable updated: Use clang-tidy instead of cppcheck (#1025)
This is an automated email from the ASF dual-hosted git repository.
tison pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new c5db264a Use clang-tidy instead of cppcheck (#1025)
c5db264a is described below
commit c5db264a0c6850ffc8f344129efed02d82c1b532
Author: Twice <tw...@gmail.com>
AuthorDate: Sun Oct 23 09:32:25 2022 +0800
Use clang-tidy instead of cppcheck (#1025)
Co-authored-by: tison <wa...@gmail.com>
---
.clang-tidy | 12 +++++++++++
.github/workflows/kvrocks.yaml | 11 +++++-----
src/commands/redis_cmd.cc | 2 +-
src/common/event_util.h | 3 +--
src/common/sha1.cc | 2 +-
src/common/util.cc | 4 ++--
src/config/config_util.cc | 2 --
src/server/worker.cc | 2 +-
src/types/geohash.cc | 4 ++--
src/types/geohash.h | 8 +++----
src/types/redis_zset.cc | 2 +-
x.py | 49 ++++++++++++++++++++++++++----------------
12 files changed, 61 insertions(+), 40 deletions(-)
diff --git a/.clang-tidy b/.clang-tidy
new file mode 100644
index 00000000..4f70482c
--- /dev/null
+++ b/.clang-tidy
@@ -0,0 +1,12 @@
+# refer to https://clang.llvm.org/extra/clang-tidy/checks/list.html
+Checks: -*, clang-analyzer-core.*, clang-analyzer-cplusplus.*, clang-analyzer-deadcode.*, clang-analyzer-nullability.*, clang-analyzer-security.*, clang-analyzer-unix.*, clang-analyzer-valist.*, cppcoreguidelines-init-variables, cppcoreguidelines-macro-usage, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-narrowing-conversions, cppcoreguidelines-no-malloc, cppcoreguidelines-prefer-member-initializer, cppcoreguidelines-special-member-functions, cppcoreguidelines-slicing, goog [...]
+
+WarningsAsErrors: clang-analyzer-*, -clang-analyzer-security.insecureAPI.rand, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-no-malloc, cppcoreguidelines-slicing, google-*, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, perfor [...]
+
+CheckOptions:
+ - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor
+ value: True
+ - key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctionsWhenCopyIsDeleted
+ value: True
+ - key: performance-move-const-arg.CheckTriviallyCopyableMove
+ value: False
diff --git a/.github/workflows/kvrocks.yaml b/.github/workflows/kvrocks.yaml
index 778c996c..0efff313 100644
--- a/.github/workflows/kvrocks.yaml
+++ b/.github/workflows/kvrocks.yaml
@@ -45,8 +45,7 @@ jobs:
- name: Install Check Tools
run: |
sudo apt update
- sudo apt install -y cppcheck
- sudo apt install -y clang-format-12
+ sudo apt install -y clang-format-12 clang-tidy-12
- name: Setup Go
uses: actions/setup-go@v3
with:
@@ -65,10 +64,12 @@ jobs:
if: always() && steps.check-format.outcome != 'success'
with:
path: clang-format.patch
- - name: Lint and check code
+ - name: Lint gocase code
+ run: ./x.py check golangci-lint
+ - name: Check cpp code via clang-tidy
run: |
- ./x.py check cppcheck
- ./x.py check golangci-lint
+ ./x.py build --skip-build
+ ./x.py check tidy -j $(nproc) --clang-tidy-path clang-tidy-12 --run-clang-tidy-path run-clang-tidy-12
build-and-test:
name: Build and test
diff --git a/src/commands/redis_cmd.cc b/src/commands/redis_cmd.cc
index 7bc0b0a8..1054c30c 100644
--- a/src/commands/redis_cmd.cc
+++ b/src/commands/redis_cmd.cc
@@ -584,7 +584,7 @@ class CommandSet : public Commander {
return Commander::Parse(args);
}
Status Execute(Server *svr, Connection *conn, std::string *output) override {
- int ret;
+ int ret = 0;
Redis::String string_db(svr->storage_, conn->GetNamespace());
rocksdb::Status s;
diff --git a/src/common/event_util.h b/src/common/event_util.h
index b63964b8..cbd26fd6 100644
--- a/src/common/event_util.h
+++ b/src/common/event_util.h
@@ -29,7 +29,7 @@
template <typename F, F *f>
struct StaticFunction {
template <typename... Ts>
- auto operator()(Ts &&...args) const -> decltype(f(std::forward<Ts>(args)...)) {
+ auto operator()(Ts &&...args) const -> decltype(f(std::forward<Ts>(args)...)) { // NOLINT
return f(std::forward<Ts>(args)...);
}
};
@@ -44,7 +44,6 @@ struct UniqueFreePtr : std::unique_ptr<T, StaticFree> {
};
struct UniqueEvbufReadln : UniqueFreePtr<char[]> {
- // cppcheck-suppress uninitMemberVar
UniqueEvbufReadln(evbuffer *buffer, evbuffer_eol_style eol_style)
: UniqueFreePtr(evbuffer_readln(buffer, &length, eol_style)) {}
diff --git a/src/common/sha1.cc b/src/common/sha1.cc
index d41626cc..d24451f9 100644
--- a/src/common/sha1.cc
+++ b/src/common/sha1.cc
@@ -188,7 +188,7 @@ void SHA1Transform(uint32_t state[5], const unsigned char buffer[64]) {
state[3] += d;
state[4] += e;
/* Wipe variables */
- a = b = c = d = e = 0;
+ a = b = c = d = e = 0; // NOLINT
#ifdef SHA1HANDSOFF
memset(block, '\0', sizeof(block));
#endif
diff --git a/src/common/util.cc b/src/common/util.cc
index 5886d5a2..5c01ae14 100644
--- a/src/common/util.cc
+++ b/src/common/util.cc
@@ -637,9 +637,9 @@ std::vector<std::string> TokenizeRedisProtocol(const std::string &value) {
}
bool IsPortInUse(int port) {
- int fd;
+ int fd = NullFD;
Status s = SockConnect("0.0.0.0", static_cast<uint32_t>(port), &fd);
- if (fd > 0) close(fd);
+ if (fd != NullFD) close(fd);
return s.IsOK();
}
diff --git a/src/config/config_util.cc b/src/config/config_util.cc
index 8d45b4b1..4e788dc5 100644
--- a/src/config/config_util.cc
+++ b/src/config/config_util.cc
@@ -137,10 +137,8 @@ StatusOr<ConfigKV> ParseConfigLine(const std::string& line) {
if (state == KEY) {
res.first = current_str;
- state = AFTER_KEY_SPACE;
} else if (state == NORMAL) {
res.second = Util::Trim(current_str, " \t\r\n\v\f\b");
- state = AFTER_VAL_SPACE;
} else if (state == QUOTED || state == ESCAPE) {
return {Status::NotOK, "config line ends unexpectedly in quoted string"};
} else if (state == ERROR) {
diff --git a/src/server/worker.cc b/src/server/worker.cc
index c4056768..17bcd8c4 100644
--- a/src/server/worker.cc
+++ b/src/server/worker.cc
@@ -101,7 +101,7 @@ void Worker::TimerCB(int, int16_t events, void *ctx) {
void Worker::newTCPConnection(evconnlistener *listener, evutil_socket_t fd, sockaddr *address, int socklen, void *ctx) {
auto worker = static_cast<Worker *>(ctx);
- int local_port = Util::GetLocalPort(fd);
+ int local_port = Util::GetLocalPort(fd); // NOLINT
DLOG(INFO) << "[worker] New connection: fd=" << fd << " from port: " << local_port << " thread #" << worker->tid_;
auto s = Util::SockSetTcpKeepalive(fd, 120);
if (!s.IsOK()) {
diff --git a/src/types/geohash.cc b/src/types/geohash.cc
index b78041f9..07b813d2 100644
--- a/src/types/geohash.cc
+++ b/src/types/geohash.cc
@@ -261,7 +261,7 @@ static void geohash_move_x(GeoHashBits *hash, int8_t d) {
uint64_t x = hash->bits & 0xaaaaaaaaaaaaaaaaULL;
uint64_t y = hash->bits & 0x5555555555555555ULL;
- uint64_t zz = 0x5555555555555555ULL >> (64 - hash->step * 2);
+ uint64_t zz = 0x5555555555555555ULL >> (64 - hash->step * 2); // NOLINT
if (d > 0) {
x = x + (zz + 1);
@@ -270,7 +270,7 @@ static void geohash_move_x(GeoHashBits *hash, int8_t d) {
x = x - (zz + 1);
}
- x &= (0xaaaaaaaaaaaaaaaaULL >> (64 - hash->step * 2));
+ x &= (0xaaaaaaaaaaaaaaaaULL >> (64 - hash->step * 2)); // NOLINT
hash->bits = (x | y);
}
diff --git a/src/types/geohash.h b/src/types/geohash.h
index 6ae8b80d..18de01ee 100644
--- a/src/types/geohash.h
+++ b/src/types/geohash.h
@@ -88,13 +88,13 @@ typedef enum {
} GeoDirection;
typedef struct {
- uint64_t bits;
- uint8_t step;
+ uint64_t bits = 0;
+ uint8_t step = 0;
} GeoHashBits;
typedef struct {
- double min;
- double max;
+ double min = 0;
+ double max = 0;
} GeoHashRange;
typedef struct {
diff --git a/src/types/redis_zset.cc b/src/types/redis_zset.cc
index fca4b2e3..63d2a9a8 100644
--- a/src/types/redis_zset.cc
+++ b/src/types/redis_zset.cc
@@ -476,7 +476,7 @@ rocksdb::Status ZSet::RangeByLex(const Slice &user_key, ZRangeLexSpec spec, std:
if (spec.count > 0 && members && members->size() >= static_cast<unsigned>(spec.count)) break;
}
- if (spec.removed && *size > 0) {
+ if (spec.removed && size && *size > 0) {
metadata.size -= *size;
std::string bytes;
metadata.Encode(&bytes);
diff --git a/x.py b/x.py
index 5d85d77d..75691e8b 100755
--- a/x.py
+++ b/x.py
@@ -30,6 +30,7 @@ from warnings import warn
CMAKE_REQUIRE_VERSION = (3, 16, 0)
CLANG_FORMAT_REQUIRED_VERSION = (12, 0, 0)
+CLANG_TIDY_REQUIRED_VERSION = (12, 0, 0)
SEMVER_REGEX = re.compile(
r"""
@@ -86,7 +87,7 @@ def check_version(current: str, required: Tuple[int, int, int], prog_name: Optio
return semver
-def build(dir: str, jobs: int, ghproxy: bool, ninja: bool, unittest: bool, compiler: str, cmake_path: str, D: List[str]) -> None:
+def build(dir: str, jobs: int, ghproxy: bool, ninja: bool, unittest: bool, compiler: str, cmake_path: str, D: List[str], skip_build: bool) -> None:
basedir = Path(__file__).parent.absolute()
find_command("autoconf", msg="autoconf is required to build jemalloc")
@@ -113,6 +114,9 @@ def build(dir: str, jobs: int, ghproxy: bool, ninja: bool, unittest: bool, compi
cmake_options += [f"-D{o}" for o in D]
run(cmake, str(basedir), *cmake_options, verbose=True, cwd=dir)
+ if skip_build:
+ return
+
target = ["kvrocks", "kvrocks2redis"]
if unittest:
target.append("unittest")
@@ -152,22 +156,24 @@ def clang_format(clang_format_path: str, fix: bool = False) -> None:
run(command, *options, *sources, verbose=True, cwd=basedir)
-def cppcheck() -> None:
- command = find_command("cppcheck", msg="cppcheck is required")
- basedir = Path(__file__).parent.absolute()
+def clang_tidy(dir: str, jobs: int, clang_tidy_path: str, run_clang_tidy_path: str) -> None:
+ # use the run-clang-tidy Python script provided by LLVM Clang
+ run_command = find_command(run_clang_tidy_path, msg="run-clang-tidy is required")
+ tidy_command = find_command(clang_tidy_path, msg="clang-tidy is required")
- options = ["-x", "c++"]
- options.append("-U__GNUC__")
- options.append("--force")
- options.append("--std=c++11")
- # we should run cmake configuration to fetch deps if we want to enable missingInclude
- options.append("--enable=warning,portability,information")
- options.append("--error-exitcode=1")
- options.append("--inline-suppr")
+ version_res = run_pipe(tidy_command, '--version').read().strip()
+ version_str = re.search(r'version\s+((?:\w|\.)+)', version_res).group(1)
- sources = ["src"]
+ check_version(version_str, CLANG_TIDY_REQUIRED_VERSION, "clang-tidy")
- run(command, *options, *sources, verbose=True, cwd=basedir)
+ if not (Path(dir) / 'compile_commands.json').exists():
+ raise RuntimeError(f"expect compile_commands.json in build directory {dir}")
+
+ basedir = Path(__file__).parent.absolute()
+
+ options = ['-p', dir, '-clang-tidy-binary', tidy_command, f'-j{jobs}']
+
+ run(run_command, *options, 'kvrocks/src/', verbose=True, cwd=basedir)
def golangci_lint() -> None:
go = find_command('go', msg='go is required for testing')
@@ -293,13 +299,17 @@ if __name__ == '__main__':
help="Check source format by clang-format")
parser_check_format.set_defaults(func=lambda **args: clang_format(**args, fix=False))
parser_check_format.add_argument('--clang-format-path', default='clang-format', help="path of clang-format used to check source")
- parser_check_cppcheck = parser_check_subparsers.add_parser(
- 'cppcheck',
- description="Check code with cppcheck (https://github.com/danmar/cppcheck)",
- help="Check code with cppcheck (https://github.com/danmar/cppcheck)",
+ parser_check_tidy = parser_check_subparsers.add_parser(
+ 'tidy',
+ description="Check code with clang-tidy",
+ help="Check code with clang-tidy",
formatter_class=ArgumentDefaultsHelpFormatter,
)
- parser_check_cppcheck.set_defaults(func=cppcheck)
+ parser_check_tidy.set_defaults(func=clang_tidy)
+ parser_check_tidy.add_argument('dir', metavar='BUILD_DIR', nargs='?', default='build', help="directory to store cmake-generated and build files")
+ parser_check_tidy.add_argument('-j', '--jobs', default=4, metavar='N', help='execute N build jobs concurrently')
+ parser_check_tidy.add_argument('--clang-tidy-path', default='clang-tidy', help="path of clang-tidy used to check source")
+ parser_check_tidy.add_argument('--run-clang-tidy-path', default='run-clang-tidy', help="path of run-clang-tidy used to check source")
parser_check_golangci_lint = parser_check_subparsers.add_parser(
'golangci-lint',
description="Check code with golangci-lint (https://golangci-lint.run/)",
@@ -322,6 +332,7 @@ if __name__ == '__main__':
parser_build.add_argument('--compiler', default='auto', choices=('auto', 'gcc', 'clang'), help="compiler used to build kvrocks")
parser_build.add_argument('--cmake-path', default='cmake', help="path of cmake binary used to build kvrocks")
parser_build.add_argument('-D', nargs='*', metavar='key=value', help='extra CMake definitions')
+ parser_build.add_argument('--skip-build', default=False, action='store_true', help='runs only the configure stage, skip the build stage')
parser_build.set_defaults(func=build)
parser_package = subparsers.add_parser(