From 1e34a0e9a8a2034159e93043f6257979b267f36f Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 28 Nov 2017 18:04:49 +0100 Subject: [PATCH] tools: add cpplint rule for NULL usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit is a suggestion for adding a rule for NULL usages in the code base. This will currently report a number of errors which could be ignored using // NOLINT (readability/null_usage) PR-URL: https://github.com/nodejs/node/pull/17373 Reviewed-By: Jon Moss Reviewed-By: Anna Henningsen Reviewed-By: Timothy Gu Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Tobias Nießen --- src/node.h | 15 +++++------ src/node_api.h | 2 +- src/node_win32_etw_provider-inl.h | 2 +- test/addons-napi/7_factory_wrap/binding.cc | 1 + .../addons-napi/test_make_callback/binding.cc | 2 ++ .../test_make_callback_recurse/binding.cc | 2 ++ tools/cpplint.py | 25 ++++++++++++++++++- tools/icu/iculslocs.cc | 19 +++++++------- 8 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/node.h b/src/node.h index 9ce8664c98e998..e6f47aa30075c4 100644 --- a/src/node.h +++ b/src/node.h @@ -501,13 +501,13 @@ extern "C" NODE_EXTERN void node_module_register(void* mod); { \ NODE_MODULE_VERSION, \ flags, \ - NULL, \ + NULL, /* NOLINT (readability/null_usage) */ \ __FILE__, \ (node::addon_register_func) (regfunc), \ - NULL, \ + NULL, /* NOLINT (readability/null_usage) */ \ NODE_STRINGIFY(modname), \ priv, \ - NULL \ + NULL /* NOLINT (readability/null_usage) */ \ }; \ NODE_C_CTOR(_register_ ## modname) { \ node_module_register(&_module); \ @@ -520,13 +520,13 @@ extern "C" NODE_EXTERN void node_module_register(void* mod); { \ NODE_MODULE_VERSION, \ flags, \ - NULL, \ + NULL, /* NOLINT (readability/null_usage) */ \ __FILE__, \ - NULL, \ + NULL, /* NOLINT (readability/null_usage) */ \ (node::addon_context_register_func) (regfunc), \ NODE_STRINGIFY(modname), \ priv, \ - NULL \ + NULL /* NOLINT (readability/null_usage) */ \ }; \ NODE_C_CTOR(_register_ ## modname) { \ node_module_register(&_module); \ @@ -534,9 +534,10 @@ extern "C" NODE_EXTERN void node_module_register(void* mod); } #define NODE_MODULE(modname, regfunc) \ - NODE_MODULE_X(modname, regfunc, NULL, 0) + NODE_MODULE_X(modname, regfunc, NULL, 0) // NOLINT (readability/null_usage) #define NODE_MODULE_CONTEXT_AWARE(modname, regfunc) \ + /* NOLINTNEXTLINE (readability/null_usage) */ \ NODE_MODULE_CONTEXT_AWARE_X(modname, regfunc, NULL, 0) /* diff --git a/src/node_api.h b/src/node_api.h index 8e5eef8a47728f..ee0ad3518e13aa 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -100,7 +100,7 @@ typedef struct { EXTERN_C_END #define NAPI_MODULE(modname, regfunc) \ - NAPI_MODULE_X(modname, regfunc, NULL, 0) + NAPI_MODULE_X(modname, regfunc, NULL, 0) // NOLINT (readability/null_usage) #define NAPI_AUTO_LENGTH SIZE_MAX diff --git a/src/node_win32_etw_provider-inl.h b/src/node_win32_etw_provider-inl.h index 3c90bee67d26f3..98a28f14f45a15 100644 --- a/src/node_win32_etw_provider-inl.h +++ b/src/node_win32_etw_provider-inl.h @@ -115,7 +115,7 @@ extern int events_enabled; DWORD status = event_write(node_provider, \ &eventDescriptor, \ 0, \ - NULL); \ + NULL); // NOLINT (readability/null_usage) \ CHECK_EQ(status, ERROR_SUCCESS); diff --git a/test/addons-napi/7_factory_wrap/binding.cc b/test/addons-napi/7_factory_wrap/binding.cc index c8fca4d536e74c..d98132457875a1 100644 --- a/test/addons-napi/7_factory_wrap/binding.cc +++ b/test/addons-napi/7_factory_wrap/binding.cc @@ -16,6 +16,7 @@ napi_value Init(napi_env env, napi_value exports) { NAPI_CALL(env, MyObject::Init(env)); NAPI_CALL(env, + // NOLINTNEXTLINE (readability/null_usage) napi_create_function(env, "exports", -1, CreateObject, NULL, &exports)); return exports; } diff --git a/test/addons-napi/test_make_callback/binding.cc b/test/addons-napi/test_make_callback/binding.cc index 4b0537ca07cf17..952dfcc1cb5bec 100644 --- a/test/addons-napi/test_make_callback/binding.cc +++ b/test/addons-napi/test_make_callback/binding.cc @@ -8,6 +8,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { const int kMaxArgs = 10; size_t argc = kMaxArgs; napi_value args[kMaxArgs]; + // NOLINTNEXTLINE (readability/null_usage) NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); NAPI_ASSERT(env, argc > 0, "Wrong number of arguments"); @@ -47,6 +48,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { napi_value Init(napi_env env, napi_value exports) { napi_value fn; NAPI_CALL(env, napi_create_function( + // NOLINTNEXTLINE (readability/null_usage) env, NULL, NAPI_AUTO_LENGTH, MakeCallback, NULL, &fn)); NAPI_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn)); return exports; diff --git a/test/addons-napi/test_make_callback_recurse/binding.cc b/test/addons-napi/test_make_callback_recurse/binding.cc index 714e44773de623..b99c583d31d9f9 100644 --- a/test/addons-napi/test_make_callback_recurse/binding.cc +++ b/test/addons-napi/test_make_callback_recurse/binding.cc @@ -7,6 +7,7 @@ namespace { napi_value MakeCallback(napi_env env, napi_callback_info info) { size_t argc = 2; napi_value args[2]; + // NOLINTNEXTLINE (readability/null_usage) NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); napi_value recv = args[0]; @@ -21,6 +22,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { napi_value Init(napi_env env, napi_value exports) { napi_value fn; NAPI_CALL(env, napi_create_function( + // NOLINTNEXTLINE (readability/null_usage) env, NULL, NAPI_AUTO_LENGTH, MakeCallback, NULL, &fn)); NAPI_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn)); return exports; diff --git a/tools/cpplint.py b/tools/cpplint.py index ca42ddeb7bc477..460c1ecbfeb02f 100644 --- a/tools/cpplint.py +++ b/tools/cpplint.py @@ -499,7 +499,6 @@ _ALT_TOKEN_REPLACEMENT_PATTERN = re.compile( r'[ =()](' + ('|'.join(_ALT_TOKEN_REPLACEMENT.keys())) + r')(?=[ (]|$)') - # These constants define types of headers for use with # _IncludeState.CheckNextIncludeOrder(). _C_SYS_HEADER = 1 @@ -526,6 +525,8 @@ # Match string that indicates we're working on a Linux Kernel file. _SEARCH_KERNEL_FILE = re.compile(r'\b(?:LINT_KERNEL_FILE)') +_NULL_TOKEN_PATTERN = re.compile(r'\bNULL\b') + _regexp_compile_cache = {} # {str, set(int)}: a map from error categories to sets of linenumbers @@ -4156,6 +4157,27 @@ def CheckAltTokens(filename, clean_lines, linenum, error): 'Use operator %s instead of %s' % ( _ALT_TOKEN_REPLACEMENT[match.group(1)], match.group(1))) +def CheckNullTokens(filename, clean_lines, linenum, error): + """Check NULL usage. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] + + # Avoid preprocessor lines + if Match(r'^\s*#', line): + return + + if line.find('/*') >= 0 or line.find('*/') >= 0: + return + + for match in _NULL_TOKEN_PATTERN.finditer(line): + error(filename, linenum, 'readability/null_usage', 2, + 'Use nullptr instead of NULL') def GetLineWidth(line): """Determines the width of the line in column positions. @@ -4294,6 +4316,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, CheckSpacingForFunctionCall(filename, clean_lines, linenum, error) CheckCheck(filename, clean_lines, linenum, error) CheckAltTokens(filename, clean_lines, linenum, error) + CheckNullTokens(filename, clean_lines, linenum, error) classinfo = nesting_state.InnermostClass() if classinfo: CheckSectionSpacing(filename, clean_lines, classinfo, linenum, error) diff --git a/tools/icu/iculslocs.cc b/tools/icu/iculslocs.cc index 485a179deb2e04..ca312b783565c4 100644 --- a/tools/icu/iculslocs.cc +++ b/tools/icu/iculslocs.cc @@ -231,14 +231,14 @@ int dumpAllButInstalledLocales(int lev, int list(const char* toBundle) { UErrorCode status = U_ZERO_ERROR; - FILE* bf = NULL; + FILE* bf = NULL; // NOLINT (readability/null_usage) - if (toBundle != NULL) { + if (toBundle != NULL) { // NOLINT (readability/null_usage) if (VERBOSE) { printf("writing to bundle %s\n", toBundle); } bf = fopen(toBundle, "wb"); - if (bf == NULL) { + if (bf == NULL) { // NOLINT (readability/null_usage) printf("ERROR: Could not open '%s' for writing.\n", toBundle); return 1; } @@ -258,6 +258,7 @@ int list(const char* toBundle) { ures_openDirect(packageName.data(), locale, &status)); ASSERT_SUCCESS(&status, "while opening the bundle"); LocalUResourceBundlePointer installedLocales( + // NOLINTNEXTLINE (readability/null_usage) ures_getByKey(bund.getAlias(), INSTALLEDLOCALES, NULL, &status)); ASSERT_SUCCESS(&status, "while fetching installed locales"); @@ -266,7 +267,7 @@ int list(const char* toBundle) { printf("Locales: %d\n", count); } - if (bf != NULL) { + if (bf != NULL) { // NOLINT (readability/null_usage) // write the HEADER fprintf(bf, "// Warning this file is automatically generated\n" @@ -310,17 +311,17 @@ int list(const char* toBundle) { UBool exists; if (localeExists(key, &exists)) { - if (bf != NULL) fclose(bf); + if (bf != NULL) fclose(bf); // NOLINT (readability/null_usage) return 1; // get out. } if (exists) { validCount++; printf("%s\n", key); - if (bf != NULL) { + if (bf != NULL) { // NOLINT (readability/null_usage) fprintf(bf, " %s {\"\"}\n", key); } } else { - if (bf != NULL) { + if (bf != NULL) { // NOLINT (readability/null_usage) fprintf(bf, "// %s {\"\"}\n", key); } if (VERBOSE) { @@ -329,7 +330,7 @@ int list(const char* toBundle) { } } - if (bf != NULL) { + if (bf != NULL) { // NOLINT (readability/null_usage) fprintf(bf, " } // %d/%d valid\n", validCount, count); // write the HEADER fprintf(bf, "}\n"); @@ -371,7 +372,7 @@ int main(int argc, const char* argv[]) { usage(); return 0; } else if (!strcmp(arg, "-l")) { - if (list(NULL)) { + if (list(NULL)) { // NOLINT (readability/null_usage) return 1; } } else if (!strcmp(arg, "-b") && (argsLeft >= 1)) {