Skip to content

Commit

Permalink
tools: add cpplint rule for NULL usage
Browse files Browse the repository at this point in the history
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: nodejs#17373
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
  • Loading branch information
danbev committed Dec 1, 2017
1 parent e32bbbf commit 12c8b4d
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 19 deletions.
15 changes: 8 additions & 7 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -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); \
Expand All @@ -520,23 +520,24 @@ 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); \
} \
}

#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)

/*
Expand Down
2 changes: 1 addition & 1 deletion src/node_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/node_win32_etw_provider-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);


Expand Down
1 change: 1 addition & 0 deletions test/addons-napi/7_factory_wrap/binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions test/addons-napi/test_make_callback/binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions test/addons-napi/test_make_callback_recurse/binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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;
Expand Down
25 changes: 24 additions & 1 deletion tools/cpplint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
19 changes: 10 additions & 9 deletions tools/icu/iculslocs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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");

Expand All @@ -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"
Expand Down Expand Up @@ -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) {
Expand All @@ -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");
Expand Down Expand Up @@ -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)) {
Expand Down

0 comments on commit 12c8b4d

Please sign in to comment.