Skip to content

Commit

Permalink
lint: misc cleanups
Browse files Browse the repository at this point in the history 
Earlier lint cleanups overlooked a couple of modules because on macOS at
the moment oclint ignores them. I noticed this when I ran `make lint-all`
on Ubuntu.
  • Loading branch information
Kurtis Rader committed Nov 5, 2016
1 parent 9886354 commit 1fb8f4e
Show file tree
Hide file tree
Showing 9 changed files with 114 additions and 93 deletions.
19 changes: 10 additions & 9 deletions osx/osx_fish_launcher.m
View file
Expand Up @@ -18,11 +18,11 @@ static void die(const char *format, ...) {
vfprintf(stderr, format, ap);
va_end(ap);
fputc('\n', stderr);

if (s_command_path[0] != '\0') {
unlink(s_command_path);
}

exit(EXIT_FAILURE);
}

Expand All @@ -31,14 +31,14 @@ static void launch_fish_with_applescript(NSString *fish_binary_path)
// load the script from a resource by fetching its URL from within our bundle
NSString *path = [[NSBundle mainBundle] pathForResource:@"launch_fish" ofType:@"scpt"];
if (! path) die("Couldn't get path to launch_fish.scpt");

NSURL *url = [NSURL fileURLWithPath:path isDirectory:NO];
if (! url) die("Couldn't get URL to launch_fish.scpt");

NSDictionary *errors = nil;
NSAppleScript *appleScript = [[NSAppleScript alloc] initWithContentsOfURL:url error:&errors];
if (! appleScript) die("Couldn't load AppleScript");

// create the first parameter
NSAppleEventDescriptor *firstParameter =
[NSAppleEventDescriptor descriptorWithString:fish_binary_path];
Expand Down Expand Up @@ -84,16 +84,17 @@ static void launch_fish_with_applescript(NSString *fish_binary_path)

/* This approach asks Terminal to open a script that we control */
int main(void) {

@autoreleasepool {
/* Get the fish executable. Make sure it's absolute. */
NSURL *fish_executable = [[NSBundle mainBundle] URLForResource:@"fish" withExtension:@"" subdirectory:@"base/bin"];
NSURL *fish_executable = [[NSBundle mainBundle] URLForResource:@"fish" withExtension:@""
subdirectory:@"base/bin"];
if (! fish_executable)
die("Could not find fish executable in bundle");

launch_fish_with_applescript([fish_executable path]);
}

/* If we succeeded, it will clean itself up */
return 0;
}
6 changes: 4 additions & 2 deletions src/builtin.cpp
View file
Expand Up @@ -3132,7 +3132,8 @@ int builtin_parse(parser_t &parser, io_streams_t &streams, wchar_t **argv)
const wcstring src = str2wcstring(&txt.at(0), txt.size());
parse_node_tree_t parse_tree;
parse_error_list_t errors;
bool success = parse_tree_from_string(src, parse_flag_include_comments, &parse_tree, &errors);
bool success = parse_tree_from_string(src, parse_flag_include_comments, &parse_tree,
&errors);
if (! success)
{
streams.out.append(L"Parsing failed:\n");
Expand All @@ -3145,7 +3146,8 @@ int builtin_parse(parser_t &parser, io_streams_t &streams, wchar_t **argv)
streams.out.append(L"(Reparsed with continue after error)\n");
parse_tree.clear();
errors.clear();
parse_tree_from_string(src, parse_flag_continue_after_error | parse_flag_include_comments, &parse_tree, &errors);
parse_tree_from_string(src, parse_flag_continue_after_error |
parse_flag_include_comments, &parse_tree, &errors);
}
const wcstring dump = parse_dump_tree(parse_tree, src);
streams.out.append(dump);
Expand Down
5 changes: 2 additions & 3 deletions src/env.cpp
View file
Expand Up @@ -581,11 +581,10 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode)
node = top;
} else if (preexisting_node != NULL) {
node = preexisting_node;

if ((var_mode & (ENV_EXPORT | ENV_UNEXPORT)) == 0) {
// use existing entry's exportv
var_mode =
preexisting_entry_exportv ? ENV_EXPORT : 0; //!OCLINT(parameter reassignment)
var_mode = //!OCLINT(parameter reassignment)
preexisting_entry_exportv ? ENV_EXPORT : 0;
}
} else {
if (!get_proc_had_barrier()) {
Expand Down
9 changes: 3 additions & 6 deletions src/fallback.cpp
View file
Expand Up @@ -203,13 +203,10 @@ size_t wcslcpy(wchar_t *dst, const wchar_t *src, size_t siz) {

// Not enough room in dst, add NUL and traverse rest of src.
if (n == 0) {
if (siz != 0) *d = '\0';
// NUL-terminate dst.
while (*s++)
;
if (siz != 0) *d = '\0'; // NUL-terminate dst
while (*s++) ; // ignore rest of src
}
return s - src - 1;
// Count does not include NUL.
return s - src - 1; // count does not include NUL
}
#endif

Expand Down
19 changes: 12 additions & 7 deletions src/fish_key_reader.cpp
View file
Expand Up @@ -206,9 +206,8 @@ static void process_input(bool continuous_mode) {
output_bind_command(bind_chars);
if (first_char_seen && !continuous_mode) {
return;
} else {
continue;
}
continue;
}

prev_tstamp = output_elapsed_time(prev_tstamp, first_char_seen);
Expand Down Expand Up @@ -307,11 +306,13 @@ int main(int argc, char **argv) {
{"help", no_argument, NULL, 'h'},
{NULL, 0, NULL, 0}};
int opt;
while ((opt = getopt_long(argc, argv, short_opts, long_opts, NULL)) != -1) {
bool error = false;
while (!error && (opt = getopt_long(argc, argv, short_opts, long_opts, NULL)) != -1) {
switch (opt) {
case 0: {
fprintf(stderr, "getopt_long() unexpectedly returned zero\n");
exit(1);
error = true;
break;
}
case 'c': {
continuous_mode = true;
Expand All @@ -320,6 +321,7 @@ int main(int argc, char **argv) {
case 'h': {
print_help("fish_key_reader", 0);
exit(0);
break;
}
case 'd': {
char *end;
Expand All @@ -332,7 +334,7 @@ int main(int argc, char **argv) {
debug_level = (int)tmp;
} else {
fwprintf(stderr, _(L"Invalid value '%s' for debug-level flag"), optarg);
exit(1);
error = true;
}
break;
}
Expand All @@ -347,16 +349,19 @@ int main(int argc, char **argv) {
debug_stack_frames = (int)tmp;
} else {
fwprintf(stderr, _(L"Invalid value '%s' for debug-stack-frames flag"), optarg);
exit(1);
error = true;
break;
}
break;
}
default: {
// We assume getopt_long() has already emitted a diagnostic msg.
exit(1);
error = true;
break;
}
}
}
if (error) return 1;

argc -= optind;
if (argc != 0) {
Expand Down
130 changes: 68 additions & 62 deletions src/fish_tests.cpp
View file
Expand Up @@ -154,19 +154,22 @@ static int chdir_set_pwd(const char *path) {
return ret;
}

// The odd formulation of these macros is to avoid "multiple unary operator" warnings from oclint
// were we to use the more natural "if (!(e)) err(..." form. We have to do this because the rules
// for the C preprocessor make it practically impossible to embed a comment in the body of a macro.
#define do_test(e) \
do { \
if (!(e)) err(L"Test failed on line %lu: %s", __LINE__, #e); \
if (e) { ; } else { err(L"Test failed on line %lu: %s", __LINE__, #e); } \
} while (0)

#define do_test_from(e, from_line) \
#define do_test_from(e, from) \
do { \
if (!(e)) err(L"Test failed on line %lu (from %lu): %s", __LINE__, from_line, #e); \
if (e) { ; } else { err(L"Test failed on line %lu (from %lu): %s", __LINE__, from, #e); } \
} while (0)

#define do_test1(e, msg) \
do { \
if (!(e)) err(L"Test failed on line %lu: %ls", __LINE__, (msg)); \
if (e) { ; } else { err(L"Test failed on line %lu: %ls", __LINE__, (msg)); } \
} while (0)

/// Test sane escapes.
Expand Down Expand Up @@ -199,7 +202,7 @@ static void test_unescape_sane() {
err(L"Should not have been able to unescape \\U110000\n");
}
if (is_wchar_ucs2()) {
// TODO: Make this work on MS Windows.
; // TODO: Make this work on MS Windows.
} else {
if (!unescape_string(L"echo \\U10FFFF", &output, UNESCAPE_DEFAULT)) {
err(L"Should have been able to unescape \\U10FFFF\n");
Expand Down Expand Up @@ -484,7 +487,7 @@ static parser_test_error_bits_t detect_argument_errors(const wcstring &src) {
return PARSER_TEST_ERROR;
}

assert(!tree.empty());
assert(!tree.empty()); //!OCLINT(multiple unary operator)
const parse_node_t *first_arg = tree.next_node_in_node_list(tree.at(0), symbol_argument, NULL);
assert(first_arg != NULL);
return parse_util_detect_errors_in_argument(*first_arg, first_arg->get_source(src));
Expand All @@ -493,7 +496,6 @@ static parser_test_error_bits_t detect_argument_errors(const wcstring &src) {
/// Test the parser.
static void test_parser() {
say(L"Testing parser");
parser_t parser;

say(L"Testing block nesting");
if (!parse_util_detect_errors(L"if; end")) {
Expand Down Expand Up @@ -627,7 +629,8 @@ static void test_parser() {
#if 0
// This is disabled since it produces a long backtrace. We should find a way to either visually
// compress the backtrace, or disable error spewing.
parser_t::principal_parser().eval(L"function recursive1 ; recursive2 ; end ; function recursive2 ; recursive1 ; end ; recursive1; ", io_chain_t(), TOP);
parser_t::principal_parser().eval(L"function recursive1 ; recursive2 ; end ; "
L"function recursive2 ; recursive1 ; end ; recursive1; ", io_chain_t(), TOP);
#endif

say(L"Testing empty function name");
Expand Down Expand Up @@ -840,8 +843,8 @@ static void test_utf82wchar(const char *src, size_t slen, const wchar_t *dst, si
const unsigned char astral_mask = 0xF0;
for (size_t i = 0; i < slen; i++) {
if ((src[i] & astral_mask) == astral_mask) {
// Astral char. We expect this conversion to just fail.
res = 0;
// Astral char. We want this conversion to fail.
res = 0; //!OCLINT(parameter reassignment)
break;
}
}
Expand Down Expand Up @@ -888,8 +891,8 @@ static void test_wchar2utf8(const wchar_t *src, size_t slen, const char *dst, si
const uint32_t astral_mask = 0xFFFF0000U;
for (size_t i = 0; i < slen; i++) {
if ((src[i] & astral_mask) != 0) {
/* astral char */
res = 0;
// Astral char. We want this conversion to fail.
res = 0; //!OCLINT(parameter reassignment)
break;
}
}
Expand Down Expand Up @@ -1137,7 +1140,8 @@ static bool expand_test(const wchar_t *in, expand_flags_t flags, ...) {
}

if (!res) {
if ((arg = va_arg(va, wchar_t *)) != 0) {
arg = va_arg(va, wchar_t *);
if (arg) {
wcstring msg = L"Expected [";
bool first = true;
for (wcstring_list_t::const_iterator it = expected.begin(), end = expected.end();
Expand Down Expand Up @@ -2207,7 +2211,6 @@ static void test_history_matches(history_search_t &search, size_t matches, unsig
size_t i;
for (i = 0; i < matches; i++) {
do_test(search.go_backwards());
wcstring item = search.current_string();
}
// do_test_from(!search.go_backwards(), from_line);
bool result = search.go_backwards();
Expand Down Expand Up @@ -2403,7 +2406,7 @@ static void trigger_or_wait_for_notification(universal_notifier_t *notifier,
universal_notifier_t::notifier_strategy_t strategy) {
switch (strategy) {
case universal_notifier_t::strategy_default: {
assert(0 && "strategy_default should be passed");
DIE("strategy_default should be passed");
break;
}
case universal_notifier_t::strategy_shmem_polling: {
Expand Down Expand Up @@ -3061,35 +3064,36 @@ static bool test_1_parse_ll2(const wcstring &src, wcstring *out_cmd, wcstring *o
out_joined_args->clear();
*out_deco = parse_statement_decoration_none;

bool result = false;
parse_node_tree_t tree;
if (parse_tree_from_string(src, parse_flag_none, &tree, NULL)) {
// Get the statement. Should only have one.
const parse_node_tree_t::parse_node_list_t stmt_nodes =
tree.find_nodes(tree.at(0), symbol_plain_statement);
if (stmt_nodes.size() != 1) {
say(L"Unexpected number of statements (%lu) found in '%ls'", stmt_nodes.size(),
src.c_str());
return false;
}
const parse_node_t &stmt = *stmt_nodes.at(0);

// Return its decoration.
*out_deco = tree.decoration_for_plain_statement(stmt);

// Return its command.
tree.command_for_plain_statement(stmt, src, out_cmd);

// Return arguments separated by spaces.
const parse_node_tree_t::parse_node_list_t arg_nodes =
tree.find_nodes(stmt, symbol_argument);
for (size_t i = 0; i < arg_nodes.size(); i++) {
if (i > 0) out_joined_args->push_back(L' ');
out_joined_args->append(arg_nodes.at(i)->get_source(src));
}
result = true;
if (!parse_tree_from_string(src, parse_flag_none, &tree, NULL)) {
return false;
}
return result;

// Get the statement. Should only have one.
const parse_node_tree_t::parse_node_list_t stmt_nodes =
tree.find_nodes(tree.at(0), symbol_plain_statement);
if (stmt_nodes.size() != 1) {
say(L"Unexpected number of statements (%lu) found in '%ls'", stmt_nodes.size(),
src.c_str());
return false;
}
const parse_node_t &stmt = *stmt_nodes.at(0);

// Return its decoration.
*out_deco = tree.decoration_for_plain_statement(stmt);

// Return its command.
tree.command_for_plain_statement(stmt, src, out_cmd);

// Return arguments separated by spaces.
const parse_node_tree_t::parse_node_list_t arg_nodes =
tree.find_nodes(stmt, symbol_argument);
for (size_t i = 0; i < arg_nodes.size(); i++) {
if (i > 0) out_joined_args->push_back(L' ');
out_joined_args->append(arg_nodes.at(i)->get_source(src));
}

return true;
}

// Test the LL2 (two token lookahead) nature of the parser by exercising the special builtin and
Expand Down Expand Up @@ -3253,29 +3257,31 @@ static wcstring_list_t separate_by_format_specifiers(const wchar_t *format) {

// Walk over the format specifier (if any).
cursor = next_specifier;
if (*cursor == '%') {
if (*cursor != '%') {
continue;
}

cursor++;
// Flag
if (wcschr(L"#0- +'", *cursor)) cursor++;
// Minimum field width
while (iswdigit(*cursor)) cursor++;
// Precision
if (*cursor == L'.') {
cursor++;
// Flag
if (wcschr(L"#0- +'", *cursor)) cursor++;
// Minimum field width
while (iswdigit(*cursor)) cursor++;
// Precision
if (*cursor == L'.') {
cursor++;
while (iswdigit(*cursor)) cursor++;
}
// Length modifier
if (!wcsncmp(cursor, L"ll", 2) || !wcsncmp(cursor, L"hh", 2)) {
cursor += 2;
} else if (wcschr(L"hljtzqL", *cursor)) {
cursor++;
}
// The format specifier itself. We allow any character except NUL.
if (*cursor != L'\0') {
cursor += 1;
}
assert(cursor <= end);
}
// Length modifier
if (!wcsncmp(cursor, L"ll", 2) || !wcsncmp(cursor, L"hh", 2)) {
cursor += 2;
} else if (wcschr(L"hljtzqL", *cursor)) {
cursor++;
}
// The format specifier itself. We allow any character except NUL.
if (*cursor != L'\0') {
cursor += 1;
}
assert(cursor <= end);
}
return result;
}
Expand Down

0 comments on commit 1fb8f4e

Please sign in to comment.