diff --git a/src/builtin.cpp b/src/builtin.cpp index 5b5fa6683..8e0f027be 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -3284,11 +3284,10 @@ int builtin_run(parser_t &parser, const wchar_t *const *argv, io_streams_t &stre cmd = (int (*)(parser_t & parser, io_streams_t & streams, const wchar_t *const *))( data ? data->func : NULL); - if (argv[1] != NULL && !builtin_handles_help(argv[0])) { - if (argv[2] == NULL && (parse_util_argument_is_help(argv[1], 0))) { - builtin_print_help(parser, streams, argv[0], streams.out); - return STATUS_BUILTIN_OK; - } + if (argv[1] != NULL && !builtin_handles_help(argv[0]) && argv[2] == NULL && + parse_util_argument_is_help(argv[1], 0)) { + builtin_print_help(parser, streams, argv[0], streams.out); + return STATUS_BUILTIN_OK; } if (data != NULL) { diff --git a/src/builtin_complete.cpp b/src/builtin_complete.cpp index 9dd180106..ec14f8ffe 100644 --- a/src/builtin_complete.cpp +++ b/src/builtin_complete.cpp @@ -286,39 +286,35 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) { } } - if (!res) { - if (condition && wcslen(condition)) { - const wcstring condition_string = condition; - parse_error_list_t errors; - if (parse_util_detect_errors(condition_string, &errors, - false /* do not accept incomplete */)) { - streams.err.append_format(L"%ls: Condition '%ls' contained a syntax error", argv[0], - condition); - for (size_t i = 0; i < errors.size(); i++) { - streams.err.append_format(L"\n%s: ", argv[0]); - streams.err.append(errors.at(i).describe(condition_string)); - } - res = true; + if (!res && condition && wcslen(condition)) { + const wcstring condition_string = condition; + parse_error_list_t errors; + if (parse_util_detect_errors(condition_string, &errors, + false /* do not accept incomplete */)) { + streams.err.append_format(L"%ls: Condition '%ls' contained a syntax error", argv[0], + condition); + for (size_t i = 0; i < errors.size(); i++) { + streams.err.append_format(L"\n%s: ", argv[0]); + streams.err.append(errors.at(i).describe(condition_string)); } + res = true; } } - if (!res) { - if (comp && wcslen(comp)) { - wcstring prefix; - if (argv[0]) { - prefix.append(argv[0]); - prefix.append(L": "); - } + if (!res && comp && wcslen(comp)) { + wcstring prefix; + if (argv[0]) { + prefix.append(argv[0]); + prefix.append(L": "); + } - wcstring err_text; - if (parser.detect_errors_in_argument_list(comp, &err_text, prefix.c_str())) { - streams.err.append_format(L"%ls: Completion '%ls' contained a syntax error\n", - argv[0], comp); - streams.err.append(err_text); - streams.err.push_back(L'\n'); - res = true; - } + wcstring err_text; + if (parser.detect_errors_in_argument_list(comp, &err_text, prefix.c_str())) { + streams.err.append_format(L"%ls: Completion '%ls' contained a syntax error\n", argv[0], + comp); + streams.err.append(err_text); + streams.err.push_back(L'\n'); + res = true; } } diff --git a/src/builtin_set_color.cpp b/src/builtin_set_color.cpp index a99882c2c..b12c39229 100644 --- a/src/builtin_set_color.cpp +++ b/src/builtin_set_color.cpp @@ -159,19 +159,17 @@ int builtin_set_color(parser_t &parser, io_streams_t &streams, wchar_t **argv) { builtin_set_color_output.clear(); output_set_writer(set_color_builtin_outputter); - if (bold) { - if (enter_bold_mode) writembs(tparm(enter_bold_mode)); + if (bold && enter_bold_mode) { + writembs(tparm(enter_bold_mode)); } - if (underline) { - if (enter_underline_mode) writembs(enter_underline_mode); + if (underline && enter_underline_mode) { + writembs(enter_underline_mode); } - if (bgcolor != NULL) { - if (bg.is_normal()) { - write_color(rgb_color_t::black(), false /* not is_fg */); - writembs(tparm(exit_attribute_mode)); - } + if (bgcolor != NULL && bg.is_normal()) { + write_color(rgb_color_t::black(), false /* not is_fg */); + writembs(tparm(exit_attribute_mode)); } if (!fg.is_none()) { @@ -188,10 +186,8 @@ int builtin_set_color(parser_t &parser, io_streams_t &streams, wchar_t **argv) { } } - if (bgcolor != NULL) { - if (!bg.is_normal() && !bg.is_reset()) { - write_color(bg, false /* not is_fg */); - } + if (bgcolor != NULL && !bg.is_normal() && !bg.is_reset()) { + write_color(bg, false /* not is_fg */); } // Restore saved writer function. diff --git a/src/common.cpp b/src/common.cpp index ceea4ba84..9740b0ce2 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1505,7 +1505,7 @@ bool list_contains_string(const wcstring_list_t &list, const wcstring &str) { } int create_directory(const wcstring &d) { - int ok = 0; + bool ok = false; struct stat buf; int stat_res = 0; @@ -1514,18 +1514,10 @@ int create_directory(const wcstring &d) { } if (stat_res == 0) { - if (S_ISDIR(buf.st_mode)) { - ok = 1; - } - } else { - if (errno == ENOENT) { - wcstring dir = wdirname(d); - if (!create_directory(dir)) { - if (!wmkdir(d, 0700)) { - ok = 1; - } - } - } + if (S_ISDIR(buf.st_mode)) ok = true; + } else if (errno == ENOENT) { + wcstring dir = wdirname(d); + if (!create_directory(dir) && !wmkdir(d, 0700)) ok = true; } return ok ? 0 : -1; diff --git a/src/complete.cpp b/src/complete.cpp index e4aac9939..b580ece60 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -670,22 +670,22 @@ void completer_t::complete_cmd(const wcstring &str_cmd, bool use_function, bool std::vector possible_comp; if (use_command) { - if (expand_string(str_cmd, &this->completions, - EXPAND_SPECIAL_FOR_COMMAND | EXPAND_FOR_COMPLETIONS | EXECUTABLES_ONLY | - this->expand_flags(), - NULL) != EXPAND_ERROR) { - if (this->wants_descriptions()) { - this->complete_cmd_desc(str_cmd); + expand_error_t result = expand_string(str_cmd, &this->completions, + EXPAND_SPECIAL_FOR_COMMAND | EXPAND_FOR_COMPLETIONS | + EXECUTABLES_ONLY | this->expand_flags(), + NULL); + if (result != EXPAND_ERROR && this->wants_descriptions()) { + this->complete_cmd_desc(str_cmd); } - } } - if (use_implicit_cd) { - if (!expand_string(str_cmd, &this->completions, - EXPAND_FOR_COMPLETIONS | DIRECTORIES_ONLY | this->expand_flags(), - NULL)) { - // Not valid as implicit cd. - } + + // WTF? This seems to be a noop. + if (use_implicit_cd && + !expand_string(str_cmd, &this->completions, + EXPAND_FOR_COMPLETIONS | DIRECTORIES_ONLY | this->expand_flags(), NULL)) { + // Not valid as implicit cd. } + if (str_cmd.find(L'/') == wcstring::npos && str_cmd.at(0) != L'~') { if (use_function) { wcstring_list_t names = function_get_names(str_cmd.at(0) == L'_'); @@ -877,11 +877,10 @@ bool completer_t::complete_param(const wcstring &scmd_orig, const wcstring &spop if (this->type() == COMPLETE_DEFAULT) { ASSERT_IS_MAIN_THREAD(); complete_load(cmd, true); - } else if (this->type() == COMPLETE_AUTOSUGGEST) { - // Maybe load this command (on the main thread). - if (!completion_autoloader.has_tried_loading(cmd)) { - iothread_perform_on_main(complete_load_no_reload, &cmd); - } + } else if (this->type() == COMPLETE_AUTOSUGGEST && + !completion_autoloader.has_tried_loading(cmd)) { + // Load this command (on the main thread). + iothread_perform_on_main(complete_load_no_reload, &cmd); } // Make a list of lists of all options that we care about. @@ -927,13 +926,12 @@ bool completer_t::complete_param(const wcstring &scmd_orig, const wcstring &spop for (option_list_t::const_iterator oiter = options.begin(); oiter != options.end(); ++oiter) { const complete_entry_opt_t *o = &*oiter; - if (o->type == option_type_single_long) { - if (param_match(o, popt) && this->condition_test(o->condition)) { - old_style_match = true; - if (o->result_mode & NO_COMMON) use_common = false; - if (o->result_mode & NO_FILES) use_files = false; - complete_from_args(str, o->comp, o->localized_desc(), o->flags); - } + if (o->type == option_type_single_long && param_match(o, popt) && + this->condition_test(o->condition)) { + old_style_match = true; + if (o->result_mode & NO_COMMON) use_common = false; + if (o->result_mode & NO_FILES) use_files = false; + complete_from_args(str, o->comp, o->localized_desc(), o->flags); } } diff --git a/src/env.cpp b/src/env.cpp index b708298b8..8c588c7b8 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -865,9 +865,7 @@ void env_push(bool new_scope) { node->next = top; node->new_scope = new_scope; - if (new_scope) { - if (local_scope_exports(top)) mark_changed_exported(); - } + if (new_scope && local_scope_exports(top)) mark_changed_exported(); top = node; } @@ -885,7 +883,7 @@ void env_pop() { } } - if (killme->new_scope) { + if (killme->new_scope) { //!OCLINT(collapsible if statements) if (killme->exportv || local_scope_exports(killme->next)) mark_changed_exported(); } diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 2bb11c521..24ba53686 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -932,17 +932,16 @@ static bool get_mac_address(unsigned char macaddr[MAC_ADDRESS_MAX_LEN], if (getifaddrs(&ifap) == 0) { for (const ifaddrs *p = ifap; p; p = p->ifa_next) { - if (p->ifa_addr && p->ifa_addr->sa_family == AF_LINK) { - if (p->ifa_name && p->ifa_name[0] && - !strcmp((const char *)p->ifa_name, interface)) { - const sockaddr_dl &sdl = *reinterpret_cast(p->ifa_addr); + bool is_af_link = p->ifa_addr && p->ifa_addr->sa_family == AF_LINK; + if (is_af_link && p->ifa_name && p->ifa_name[0] && + !strcmp((const char *)p->ifa_name, interface)) { + const sockaddr_dl &sdl = *reinterpret_cast(p->ifa_addr); - size_t alen = sdl.sdl_alen; - if (alen > MAC_ADDRESS_MAX_LEN) alen = MAC_ADDRESS_MAX_LEN; - memcpy(macaddr, sdl.sdl_data + sdl.sdl_nlen, alen); - ok = true; - break; - } + size_t alen = sdl.sdl_alen; + if (alen > MAC_ADDRESS_MAX_LEN) alen = MAC_ADDRESS_MAX_LEN; + memcpy(macaddr, sdl.sdl_data + sdl.sdl_nlen, alen); + ok = true; + break; } } freeifaddrs(ifap); @@ -1033,12 +1032,11 @@ class universal_notifier_shmem_poller_t : public universal_notifier_t { } // Set the size, if it's too small. - if (!errored && size < (off_t)sizeof(universal_notifier_shmem_t)) { - if (ftruncate(fd, sizeof(universal_notifier_shmem_t)) < 0) { - int err = errno; - report_error(err, L"Unable to truncate shared memory object with path '%s'", path); - errored = true; - } + bool set_size = !errored && size < (off_t)sizeof(universal_notifier_shmem_t); + if (set_size && ftruncate(fd, sizeof(universal_notifier_shmem_t)) < 0) { + int err = errno; + report_error(err, L"Unable to truncate shared memory object with path '%s'", path); + errored = true; } // Memory map the region. @@ -1237,10 +1235,12 @@ class universal_notifier_named_pipe_t : public universal_notifier_t { int fd = wopen_cloexec(vars_path, O_RDWR | O_NONBLOCK, 0600); if (fd < 0 && errno == ENOENT) { // File doesn't exist, try creating it. - if (mkfifo(narrow_path.c_str(), 0600) >= 0) { + int mkfifo_status = mkfifo(narrow_path.c_str(), 0600); + if (mkfifo_status != -1) { fd = wopen_cloexec(vars_path, O_RDWR | O_NONBLOCK, 0600); } } + if (fd < 0) { // Maybe open failed, maybe mkfifo failed. int err = errno; @@ -1316,11 +1316,9 @@ class universal_notifier_named_pipe_t : public universal_notifier_t { // it back. Nobody is expected to read it except us. int pid_nbo = htonl(getpid()); ssize_t amt_written = write(this->pipe_fd, &pid_nbo, sizeof pid_nbo); - if (amt_written < 0) { - if (errno == EWOULDBLOCK || errno == EAGAIN) { - // Very unsual: the pipe is full! - drain_excessive_data(); - } + if (amt_written < 0 && errno == EWOULDBLOCK || errno == EAGAIN) { + // Very unsual: the pipe is full! + drain_excessive_data(); } // Now schedule a read for some time in the future. diff --git a/src/exec.cpp b/src/exec.cpp index 777f8e354..b37d4f835 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -353,7 +353,7 @@ static void internal_exec_helper(parser_t &parser, const wcstring &def, node_off // foreground process group, we don't use posix_spawn if we're going to foreground the process. (If // we use fork(), we can call tcsetpgrp after the fork, before the exec, and avoid the race). static bool can_use_posix_spawn_for_job(const job_t *job, const process_t *process) { - if (job_get_flag(job, JOB_CONTROL)) { + if (job_get_flag(job, JOB_CONTROL)) { //!OCLINT(collapsible if statements) // We are going to use job control; therefore when we launch this job it will get its own // process group ID. But will it be foregrounded? if (job_get_flag(job, JOB_TERMINAL) && job_get_flag(job, JOB_FOREGROUND)) { @@ -913,45 +913,42 @@ void exec_job(parser_t &parser, job_t *j) { // output, so that we can truncate the file. Does not apply to /dev/null. bool must_fork = redirection_is_to_real_file(stdout_io.get()) || redirection_is_to_real_file(stderr_io.get()); - if (!must_fork) { - if (p->next == NULL) { - const bool stdout_is_to_buffer = - stdout_io && stdout_io->io_mode == IO_BUFFER; - const bool no_stdout_output = stdout_buffer.empty(); - const bool no_stderr_output = stderr_buffer.empty(); + if (!must_fork && p->next == NULL) { + const bool stdout_is_to_buffer = stdout_io && stdout_io->io_mode == IO_BUFFER; + const bool no_stdout_output = stdout_buffer.empty(); + const bool no_stderr_output = stderr_buffer.empty(); - if (no_stdout_output && no_stderr_output) { - // The builtin produced no output and is not inside of a pipeline. No - // need to fork or even output anything. - debug(3, L"Skipping fork: no output for internal builtin '%ls'", - p->argv0()); - fork_was_skipped = true; - } else if (no_stderr_output && stdout_is_to_buffer) { - // The builtin produced no stderr, and its stdout is going to an - // internal buffer. There is no need to fork. This helps out the - // performance quite a bit in complex completion code. - debug(3, L"Skipping fork: buffered output for internal builtin '%ls'", - p->argv0()); + if (no_stdout_output && no_stderr_output) { + // The builtin produced no output and is not inside of a pipeline. No + // need to fork or even output anything. + debug(3, L"Skipping fork: no output for internal builtin '%ls'", + p->argv0()); + fork_was_skipped = true; + } else if (no_stderr_output && stdout_is_to_buffer) { + // The builtin produced no stderr, and its stdout is going to an + // internal buffer. There is no need to fork. This helps out the + // performance quite a bit in complex completion code. + debug(3, L"Skipping fork: buffered output for internal builtin '%ls'", + p->argv0()); - io_buffer_t *io_buffer = static_cast(stdout_io.get()); - const std::string res = wcs2string(builtin_io_streams->out.buffer()); + io_buffer_t *io_buffer = static_cast(stdout_io.get()); + const std::string res = wcs2string(builtin_io_streams->out.buffer()); - io_buffer->out_buffer_append(res.data(), res.size()); - fork_was_skipped = true; - } else if (stdout_io.get() == NULL && stderr_io.get() == NULL) { - // We are writing to normal stdout and stderr. Just do it - no need to - // fork. - debug(3, L"Skipping fork: ordinary output for internal builtin '%ls'", - p->argv0()); - const std::string outbuff = wcs2string(stdout_buffer); - const std::string errbuff = wcs2string(stderr_buffer); - bool builtin_io_done = do_builtin_io(outbuff.data(), outbuff.size(), - errbuff.data(), errbuff.size()); - if (!builtin_io_done && errno != EPIPE) { - show_stackframe(L'E'); - } - fork_was_skipped = true; + io_buffer->out_buffer_append(res.data(), res.size()); + fork_was_skipped = true; + } else if (stdout_io.get() == NULL && stderr_io.get() == NULL) { + // We are writing to normal stdout and stderr. Just do it - no need to + // fork. + debug(3, L"Skipping fork: ordinary output for internal builtin '%ls'", + p->argv0()); + const std::string outbuff = wcs2string(stdout_buffer); + const std::string errbuff = wcs2string(stderr_buffer); + bool builtin_io_done = do_builtin_io(outbuff.data(), outbuff.size(), + errbuff.data(), errbuff.size()); + if (!builtin_io_done && errno != EPIPE) { + show_stackframe(L'E'); } + fork_was_skipped = true; } } diff --git a/src/expand.cpp b/src/expand.cpp index 040216c16..200ed98d4 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -225,10 +225,8 @@ static bool match_pid(const wcstring &cmd, const wchar_t *proc, size_t *offset) const wcstring base_cmd = wbasename(cmd); bool result = string_prefixes_string(proc, base_cmd); - if (result) { - // It's a match. Return the offset within the full command. - if (offset) *offset = cmd.size() - base_cmd.size(); - } + // It's a match. Return the offset within the full command. + if (result && offset) *offset = cmd.size() - base_cmd.size(); return result; } @@ -632,13 +630,11 @@ static bool expand_pid(const wcstring &instr_with_sep, expand_flags_t flags, const size_t prev_count = out->size(); find_process(in + 1, flags, out); - if (prev_count == out->size()) { - if (!(flags & EXPAND_FOR_COMPLETIONS)) { - // We failed to find anything. - append_syntax_error(errors, 1, FAILED_EXPANSION_PROCESS_ERR_MSG, - escape(in + 1, ESCAPE_NO_QUOTED).c_str()); - return false; - } + if (prev_count == out->size() && !(flags & EXPAND_FOR_COMPLETIONS)) { + // We failed to find anything. + append_syntax_error(errors, 1, FAILED_EXPANSION_PROCESS_ERR_MSG, + escape(in + 1, ESCAPE_NO_QUOTED).c_str()); + return false; } return true; @@ -1025,21 +1021,19 @@ static expand_error_t expand_brackets(const wcstring &instr, expand_flags_t flag tot_len = length_preceding_brackets + length_following_brackets; item_begin = bracket_begin + 1; for (const wchar_t *pos = (bracket_begin + 1); true; pos++) { - if (bracket_count == 0) { - if ((*pos == BRACKET_SEP) || (pos == bracket_end)) { - assert(pos >= item_begin); - size_t item_len = pos - item_begin; + if (bracket_count == 0 && ((*pos == BRACKET_SEP) || (pos == bracket_end))) { + assert(pos >= item_begin); + size_t item_len = pos - item_begin; - wcstring whole_item; - whole_item.reserve(tot_len + item_len + 2); - whole_item.append(in, length_preceding_brackets); - whole_item.append(item_begin, item_len); - whole_item.append(bracket_end + 1); - expand_brackets(whole_item, flags, out, errors); + wcstring whole_item; + whole_item.reserve(tot_len + item_len + 2); + whole_item.append(in, length_preceding_brackets); + whole_item.append(item_begin, item_len); + whole_item.append(bracket_end + 1); + expand_brackets(whole_item, flags, out, errors); - item_begin = pos + 1; - if (pos == bracket_end) break; - } + item_begin = pos + 1; + if (pos == bracket_end) break; } if (*pos == BRACKET_BEGIN) { @@ -1517,19 +1511,17 @@ expand_error_t expand_string(const wcstring &input, std::vector *o bool expand_one(wcstring &string, expand_flags_t flags, parse_error_list_t *errors) { std::vector completions; - bool result = false; - if ((!(flags & EXPAND_FOR_COMPLETIONS)) && expand_is_clean(string)) { + if (!(flags & EXPAND_FOR_COMPLETIONS) && expand_is_clean(string)) { return true; } - if (expand_string(string, &completions, flags | EXPAND_NO_DESCRIPTIONS, errors)) { - if (completions.size() == 1) { - string = completions.at(0).completion; - result = true; - } + if (expand_string(string, &completions, flags | EXPAND_NO_DESCRIPTIONS, errors) && + completions.size() == 1) { + string = completions.at(0).completion; + return true; } - return result; + return false; } // https://github.com/fish-shell/fish-shell/issues/367 diff --git a/src/function.cpp b/src/function.cpp index 34dbd85cd..6ca086aba 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -311,8 +311,8 @@ wcstring_list_t function_get_names(int get_hidden) { const wcstring &name = iter->first; // Maybe skip hidden. - if (!get_hidden) { - if (name.empty() || name.at(0) == L'_') continue; + if (!get_hidden && (name.empty() || name.at(0) == L'_')) { + continue; } names.insert(name); } diff --git a/src/highlight.cpp b/src/highlight.cpp index 08ca3ada0..1f1dcac52 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -224,19 +224,16 @@ static bool is_potential_cd_path(const wcstring &path, const wcstring &working_d bool plain_statement_get_expanded_command(const wcstring &src, const parse_node_tree_t &tree, const parse_node_t &plain_statement, wcstring *out_cmd) { assert(plain_statement.type == symbol_plain_statement); - bool result = false; - // Get the command. + // Get the command. Try expanding it. If we cannot, it's an error. wcstring cmd; - if (tree.command_for_plain_statement(plain_statement, src, &cmd)) { - // Try expanding it. If we cannot, it's an error. - if (expand_one(cmd, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES | EXPAND_SKIP_JOBS)) { - // Success, return the expanded string by reference. - out_cmd->swap(cmd); - result = true; - } + if (tree.command_for_plain_statement(plain_statement, src, &cmd) && + expand_one(cmd, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES | EXPAND_SKIP_JOBS)) { + // Success, return the expanded string by reference. + out_cmd->swap(cmd); + return true; } - return result; + return false; } rgb_color_t highlight_get_color(highlight_spec_t highlight, bool is_background) { @@ -297,8 +294,6 @@ static bool has_expand_reserved(const wcstring &str) { // (as a copied node), if any. This is used by autosuggestions. static bool autosuggest_parse_command(const wcstring &buff, wcstring *out_expanded_command, parse_node_t *out_last_arg) { - bool result = false; - // Parse the buffer. parse_node_tree_t parse_tree; parse_tree_from_string(buff, @@ -308,21 +303,17 @@ static bool autosuggest_parse_command(const wcstring &buff, wcstring *out_expand // Find the last statement. const parse_node_t *last_statement = parse_tree.find_last_node_of_type(symbol_plain_statement, NULL); - if (last_statement != NULL) { - if (plain_statement_get_expanded_command(buff, parse_tree, *last_statement, - out_expanded_command)) { - // We got it. - result = true; - - // Find the last argument. If we don't get one, return an invalid node. - const parse_node_t *last_arg = - parse_tree.find_last_node_of_type(symbol_argument, last_statement); - if (last_arg != NULL) { - *out_last_arg = *last_arg; - } + if (last_statement != NULL && plain_statement_get_expanded_command( + buff, parse_tree, *last_statement, out_expanded_command)) { + // Find the last argument. If we don't get one, return an invalid node. + const parse_node_t *last_arg = + parse_tree.find_last_node_of_type(symbol_argument, last_statement); + if (last_arg != NULL) { + *out_last_arg = *last_arg; } + return true; } - return result; + return false; } bool autosuggest_validate_from_history(const history_item_t &item, @@ -1163,18 +1154,15 @@ const highlighter_t::color_array_t &highlighter_t::highlight() { // backspacing (and the cursor is just beyond the last token), we may still underline // it. if (this->cursor_pos >= node.source_start && - this->cursor_pos - node.source_start <= node.source_length) { - // See if this is a valid path. - if (node_is_potential_path(buff, node, working_directory)) { - // It is, underline it. - for (size_t i = node.source_start; i < node.source_start + node.source_length; - i++) { - // Don't color highlight_spec_error because it looks dorky. For example, - // trying to cd into a non-directory would show an underline and also red. - if (highlight_get_primary(this->color_array.at(i)) != - highlight_spec_error) { - this->color_array.at(i) |= highlight_modifier_valid_path; - } + this->cursor_pos - node.source_start <= node.source_length && + node_is_potential_path(buff, node, working_directory)) { + // It is, underline it. + for (size_t i = node.source_start; i < node.source_start + node.source_length; + i++) { + // Don't color highlight_spec_error because it looks dorky. For example, + // trying to cd into a non-directory would show an underline and also red. + if (highlight_get_primary(this->color_array.at(i)) != highlight_spec_error) { + this->color_array.at(i) |= highlight_modifier_valid_path; } } } diff --git a/src/output.cpp b/src/output.cpp index 6287b4329..03c1f9fa5 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -272,12 +272,8 @@ void set_color(rgb_color_t c, rgb_color_t c2) { } // Lastly, we set bold mode and underline mode correctly. - if ((enter_bold_mode != 0) && (strlen(enter_bold_mode) > 0) && !bg_set) { - if (is_bold && !was_bold) { - if (enter_bold_mode) { - writembs(tparm(enter_bold_mode)); - } - } + if (is_bold && !was_bold && enter_bold_mode && strlen(enter_bold_mode) > 0 && !bg_set) { + writembs(tparm(enter_bold_mode)); was_bold = is_bold; } diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index a4a87c99e..b52dbfd7f 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -1208,12 +1208,10 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t // Get terminal modes. struct termios tmodes = {}; - if (shell_is_interactive()) { - if (tcgetattr(STDIN_FILENO, &tmodes)) { - // Need real error handling here. - wperror(L"tcgetattr"); - return parse_execution_errored; - } + if (shell_is_interactive() && tcgetattr(STDIN_FILENO, &tmodes)) { + // Need real error handling here. + wperror(L"tcgetattr"); + return parse_execution_errored; } // Increment the eval_level for the duration of this command. diff --git a/src/parse_productions.cpp b/src/parse_productions.cpp index 3c1fcbcb2..0455b50fa 100644 --- a/src/parse_productions.cpp +++ b/src/parse_productions.cpp @@ -500,11 +500,9 @@ const production_t *parse_productions::production_for_token(parse_token_type_t n PARSE_ASSERT(resolver != NULL); const production_t *result = resolver(input1, input2, out_tag); - if (result == NULL) { - if (log_it) { - fprintf(stderr, "Node type '%ls' has no production for input '%ls' (in %s)\n", - token_type_description(node_type), input1.describe().c_str(), __FUNCTION__); - } + if (result == NULL && log_it) { + fprintf(stderr, "Node type '%ls' has no production for input '%ls' (in %s)\n", + token_type_description(node_type), input1.describe().c_str(), __FUNCTION__); } return result; diff --git a/src/parse_tree.cpp b/src/parse_tree.cpp index b524f59fe..350eff9ca 100644 --- a/src/parse_tree.cpp +++ b/src/parse_tree.cpp @@ -1370,13 +1370,11 @@ const parse_node_t *parse_node_tree_t::find_last_node_of_type(parse_token_type_t size_t idx = this->size(); while (idx--) { const parse_node_t &node = this->at(idx); - if (node.type == type) { - // Types match. Check if it has the right parent. - if (parent == NULL || node_has_ancestor(*this, node, *parent)) { - // Success - result = &node; - break; - } + bool expected_type = (node.type == type); + if (expected_type && (parent == NULL || node_has_ancestor(*this, node, *parent))) { + // The types match and it has the right parent. + result = &node; + break; } } return result; diff --git a/src/parse_util.cpp b/src/parse_util.cpp index 1cd4f895c..ff0f01883 100644 --- a/src/parse_util.cpp +++ b/src/parse_util.cpp @@ -471,7 +471,6 @@ void parse_util_get_parameter_info(const wcstring &cmd, const size_t pos, wchar_ size_t *offset, enum token_type *out_type) { size_t prev_pos = 0; wchar_t last_quote = '\0'; - int unfinished; tokenizer_t tok(cmd.c_str(), TOK_ACCEPT_UNFINISHED | TOK_SQUASH_ERRORS); tok_t token; @@ -488,30 +487,25 @@ void parse_util_get_parameter_info(const wcstring &cmd, const size_t pos, wchar_ wchar_t *cmd_tmp = wcsdup(cmd.c_str()); cmd_tmp[pos] = 0; size_t cmdlen = wcslen(cmd_tmp); - unfinished = (cmdlen == 0); - if (!unfinished) { - unfinished = (quote != 0); - - if (!unfinished) { - if (wcschr(L" \t\n\r", cmd_tmp[cmdlen - 1]) != 0) { - if ((cmdlen == 1) || (cmd_tmp[cmdlen - 2] != L'\\')) { - unfinished = 1; - } - } + bool finished = (cmdlen != 0); + if (finished) { + finished = (quote == NULL); + if (finished && wcschr(L" \t\n\r", cmd_tmp[cmdlen - 1]) == 0) { + finished = (cmdlen == 1) || (cmd_tmp[cmdlen - 2] != L'\\'); } } if (quote) *quote = last_quote; if (offset != 0) { - if (!unfinished) { + if (finished) { while ((cmd_tmp[prev_pos] != 0) && (wcschr(L";|", cmd_tmp[prev_pos]) != 0)) prev_pos++; - *offset = prev_pos; } else { *offset = pos; } } + free(cmd_tmp); } @@ -927,22 +921,21 @@ static parser_test_error_bits_t detect_dollar_cmdsub_errors(size_t arg_src_offse parse_error_list_t *out_errors) { parser_test_error_bits_t result_bits = 0; wcstring unescaped_arg_src; - if (unescape_string(arg_src, &unescaped_arg_src, UNESCAPE_SPECIAL)) { - if (!unescaped_arg_src.empty()) { - wchar_t last = unescaped_arg_src.at(unescaped_arg_src.size() - 1); - if (last == VARIABLE_EXPAND) { - result_bits |= PARSER_TEST_ERROR; - if (out_errors != NULL) { - wcstring subcommand_first_token = tok_first(cmdsubst_src); - if (subcommand_first_token.empty()) { - // e.g. $(). Report somthing. - subcommand_first_token = L"..."; - } - append_syntax_error( - out_errors, - arg_src_offset + arg_src.size() - 1, // global position of the dollar - ERROR_BAD_VAR_SUBCOMMAND1, truncate_string(subcommand_first_token).c_str()); + if (unescape_string(arg_src, &unescaped_arg_src, UNESCAPE_SPECIAL) && + !unescaped_arg_src.empty()) { + wchar_t last = unescaped_arg_src.at(unescaped_arg_src.size() - 1); + if (last == VARIABLE_EXPAND) { + result_bits |= PARSER_TEST_ERROR; + if (out_errors != NULL) { + wcstring subcommand_first_token = tok_first(cmdsubst_src); + if (subcommand_first_token.empty()) { + // e.g. $(). Report somthing. + subcommand_first_token = L"..."; } + append_syntax_error( + out_errors, + arg_src_offset + arg_src.size() - 1, // global position of the dollar + ERROR_BAD_VAR_SUBCOMMAND1, truncate_string(subcommand_first_token).c_str()); } } } diff --git a/src/postfork.cpp b/src/postfork.cpp index 5f15f9954..792ee2917 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -66,7 +66,9 @@ int set_child_group(job_t *j, process_t *p, int print_errors) { j->pgid = p->pid; } - if (setpgid(p->pid, j->pgid)) { + if (setpgid(p->pid, j->pgid)) { //!OCLINT(collapsible if statements) + // TODO: Figure out why we're testing whether the pgid is correct after attempting to + // set it failed. This was added in commit 4e912ef8 from 2012-02-27. if (getpgid(p->pid) != j->pgid && print_errors) { char pid_buff[128]; char job_id_buff[128]; @@ -95,7 +97,8 @@ int set_child_group(job_t *j, process_t *p, int print_errors) { } if (job_get_flag(j, JOB_TERMINAL) && job_get_flag(j, JOB_FOREGROUND)) { - if (tcsetpgrp(0, j->pgid) && print_errors) { + int result = tcsetpgrp(0, j->pgid); + if (result == -1 && print_errors) { char job_id_buff[64]; char command_buff[64]; format_long_safe(job_id_buff, j->job_id); @@ -485,20 +488,14 @@ void safe_report_exec_error(int err, const char *actual_cmd, const char *const * /// allocate memory, etc. bool do_builtin_io(const char *out, size_t outlen, const char *err, size_t errlen) { bool success = true; - if (out && outlen) { - if (write_loop(STDOUT_FILENO, out, outlen) < 0) { - int e = errno; - debug_safe(0, "Error while writing to stdout"); - safe_perror("write_loop"); - success = false; - errno = e; - } + if (out && outlen && write_loop(STDOUT_FILENO, out, outlen) < 0) { + int e = errno; + debug_safe(0, "Error while writing to stdout"); + safe_perror("write_loop"); + success = false; + errno = e; } - if (err && errlen) { - if (write_loop(STDERR_FILENO, err, errlen) < 0) { - success = false; - } - } + if (err && errlen && write_loop(STDERR_FILENO, err, errlen) < 0) success = false; return success; } diff --git a/src/print_help.cpp b/src/print_help.cpp index 03ae38593..c19a51dfc 100644 --- a/src/print_help.cpp +++ b/src/print_help.cpp @@ -16,9 +16,7 @@ void print_help(const char *c, int fd) { char cmd[CMD_LEN]; int printed = snprintf(cmd, CMD_LEN, "fish -c '__fish_print_help %s >&%d'", c, fd); - if (printed < CMD_LEN) { - if ((system(cmd) == -1)) { - write_loop(2, HELP_ERR, strlen(HELP_ERR)); - } + if (printed < CMD_LEN && system(cmd) == -1) { + write_loop(2, HELP_ERR, strlen(HELP_ERR)); } } diff --git a/src/proc.cpp b/src/proc.cpp index d3e7b733d..f415df14d 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -257,13 +257,9 @@ int job_signal(job_t *j, int signal) { res = killpg(j->pgid, signal); } else { for (process_t *p = j->first_process; p; p = p->next) { - if (!p->completed) { - if (p->pid) { - if (kill(p->pid, signal)) { - res = -1; - break; - } - } + if (!p->completed && p->pid && kill(p->pid, signal)) { + res = -1; + break; } } } @@ -312,10 +308,8 @@ static void handle_child_status(pid_t pid, int status) { for (p = j->first_process; p; p = p->next) { if (pid == p->pid) { mark_process_status(p, status); - if (p->completed && prev != 0) { - if (!prev->completed && prev->pid) { - kill(prev->pid, SIGPIPE); - } + if (p->completed && prev && !prev->completed && prev->pid) { + kill(prev->pid, SIGPIPE); } found_proc = true; break; @@ -568,61 +562,57 @@ int job_reap(bool allow_interactive) { proc_fire_event(L"PROCESS_EXIT", EVENT_EXIT, p->pid, (WIFSIGNALED(s) ? -1 : WEXITSTATUS(s))); - if (WIFSIGNALED(s)) { - // Ignore signal SIGPIPE.We issue it ourselves to the pipe writer when the pipe - // reader dies. - if (WTERMSIG(s) != SIGPIPE) { - int proc_is_job = ((p == j->first_process) && (p->next == 0)); - if (proc_is_job) job_set_flag(j, JOB_NOTIFIED, 1); - if (!job_get_flag(j, JOB_SKIP_NOTIFICATION)) { - // Print nothing if we get SIGINT in the foreground process group, to avoid - // spamming obvious stuff on the console (#1119). If we get SIGINT for the - // foreground process, assume the user typed ^C and can see it working. It's - // possible they didn't, and the signal was delivered via pkill, etc., but - // the SIGINT/SIGTERM distinction is precisely to allow INT to be from a UI - // and TERM to be programmatic, so this assumption is keeping with the - // design of signals. If echoctl is on, then the terminal will have written - // ^C to the console. If off, it won't have. We don't echo ^C either way, so - // as to respect the user's preference. - if (WTERMSIG(p->status) != SIGINT || !job_get_flag(j, JOB_FOREGROUND)) { - if (proc_is_job) { - // We want to report the job number, unless it's the only job, in - // which case we don't need to. - const wcstring job_number_desc = - (job_count == 1) ? wcstring() - : format_string(L"Job %d, ", j->job_id); - fwprintf(stdout, - _(L"%ls: %ls\'%ls\' terminated by signal %ls (%ls)"), - program_name, job_number_desc.c_str(), - truncate_command(j->command()).c_str(), - sig2wcs(WTERMSIG(p->status)), - signal_get_desc(WTERMSIG(p->status))); - } else { - const wcstring job_number_desc = - (job_count == 1) ? wcstring() - : format_string(L"from job %d, ", j->job_id); - fwprintf(stdout, _(L"%ls: Process %d, \'%ls\' %ls\'%ls\' " - L"terminated by signal %ls (%ls)"), - program_name, p->pid, p->argv0(), job_number_desc.c_str(), - truncate_command(j->command()).c_str(), - sig2wcs(WTERMSIG(p->status)), - signal_get_desc(WTERMSIG(p->status))); - } - - if (cur_term != NULL) - tputs(clr_eol, 1, &writeb); - else - fwprintf(stdout, - L"\x1b[K"); // no term set up - do clr_eol manually - - fwprintf(stdout, L"\n"); + // Ignore signal SIGPIPE.We issue it ourselves to the pipe writer when the pipe + // reader dies. + if (WIFSIGNALED(s) && WTERMSIG(s) != SIGPIPE) { + int proc_is_job = ((p == j->first_process) && (p->next == 0)); + if (proc_is_job) job_set_flag(j, JOB_NOTIFIED, 1); + if (!job_get_flag(j, JOB_SKIP_NOTIFICATION)) { + // Print nothing if we get SIGINT in the foreground process group, to avoid + // spamming obvious stuff on the console (#1119). If we get SIGINT for the + // foreground process, assume the user typed ^C and can see it working. It's + // possible they didn't, and the signal was delivered via pkill, etc., but + // the SIGINT/SIGTERM distinction is precisely to allow INT to be from a UI + // and TERM to be programmatic, so this assumption is keeping with the + // design of signals. If echoctl is on, then the terminal will have written + // ^C to the console. If off, it won't have. We don't echo ^C either way, so + // as to respect the user's preference. + if (WTERMSIG(p->status) != SIGINT || !job_get_flag(j, JOB_FOREGROUND)) { + if (proc_is_job) { + // We want to report the job number, unless it's the only job, in + // which case we don't need to. + const wcstring job_number_desc = + (job_count == 1) ? wcstring() + : format_string(L"Job %d, ", j->job_id); + fwprintf(stdout, _(L"%ls: %ls\'%ls\' terminated by signal %ls (%ls)"), + program_name, job_number_desc.c_str(), + truncate_command(j->command()).c_str(), + sig2wcs(WTERMSIG(p->status)), + signal_get_desc(WTERMSIG(p->status))); + } else { + const wcstring job_number_desc = + (job_count == 1) ? wcstring() + : format_string(L"from job %d, ", j->job_id); + fwprintf(stdout, _(L"%ls: Process %d, \'%ls\' %ls\'%ls\' " + L"terminated by signal %ls (%ls)"), + program_name, p->pid, p->argv0(), job_number_desc.c_str(), + truncate_command(j->command()).c_str(), + sig2wcs(WTERMSIG(p->status)), + signal_get_desc(WTERMSIG(p->status))); } - found = 1; - } - // Clear status so it is not reported more than once. - p->status = 0; + if (cur_term != NULL) + tputs(clr_eol, 1, &writeb); + else + fwprintf(stdout, L"\x1b[K"); // no term set up - do clr_eol manually + + fwprintf(stdout, L"\n"); + } + found = 1; } + + // Clear status so it is not reported more than once. + p->status = 0; } } @@ -803,13 +793,10 @@ static bool terminal_give_to_job(job_t *j, int cont) { return false; } - if (cont) { - if (tcsetattr(0, TCSADRAIN, &j->tmodes)) { - debug(1, _(L"Could not send job %d ('%ls') to foreground"), j->job_id, - j->command_wcstr()); - wperror(L"tcsetattr"); - return false; - } + if (cont && tcsetattr(0, TCSADRAIN, &j->tmodes)) { + debug(1, _(L"Could not send job %d ('%ls') to foreground"), j->job_id, j->command_wcstr()); + wperror(L"tcsetattr"); + return false; } return true; } @@ -944,15 +931,13 @@ void job_continue(job_t *j, bool cont) { process_t *p = j->first_process; while (p->next) p = p->next; - if (WIFEXITED(p->status) || WIFSIGNALED(p->status)) { - // Mark process status only if we are in the foreground and the last process in a - // pipe, and it is not a short circuited builtin. - if (p->pid) { - int status = proc_format_status(p->status); - // wprintf(L"setting status %d for %ls\n", job_get_flag( j, JOB_NEGATE - // )?!status:status, j->command); - proc_set_last_status(job_get_flag(j, JOB_NEGATE) ? !status : status); - } + // Mark process status only if we are in the foreground and the last process in a pipe, + // and it is not a short circuited builtin. + if ((WIFEXITED(p->status) || WIFSIGNALED(p->status)) && p->pid) { + int status = proc_format_status(p->status); + // wprintf(L"setting status %d for %ls\n", job_get_flag( j, JOB_NEGATE + // )?!status:status, j->command); + proc_set_last_status(job_get_flag(j, JOB_NEGATE) ? !status : status); } } diff --git a/src/reader.cpp b/src/reader.cpp index 2b6b82ccb..e4c04bad0 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -697,14 +697,13 @@ void reader_write_title(const wcstring &cmd, bool reset_cursor_position) { wcstring_list_t lst; proc_push_interactive(0); - if (exec_subshell(fish_title_command, lst, false /* do not apply exit status */) != -1) { - if (!lst.empty()) { - writestr(L"\x1b]0;"); - for (size_t i = 0; i < lst.size(); i++) { - writestr(lst.at(i).c_str()); - } - writestr(L"\7"); + if (exec_subshell(fish_title_command, lst, false /* ignore exit status */) != -1 && + !lst.empty()) { + writestr(L"\x1b]0;"); + for (size_t i = 0; i < lst.size(); i++) { + writestr(lst.at(i).c_str()); } + writestr(L"\7"); } proc_pop_interactive(); set_color(rgb_color_t::reset(), rgb_color_t::reset()); @@ -1531,10 +1530,8 @@ static bool check_for_orphaned_process(unsigned long loop_count, pid_t shell_pgi // Try kill-0'ing the process whose pid corresponds to our process group ID. It's possible this // will fail because we don't have permission to signal it. But more likely it will fail because // it no longer exists, and we are orphaned. - if (loop_count % 64 == 0) { - if (kill(shell_pgid, 0) < 0 && errno == ESRCH) { - we_think_we_are_orphaned = true; - } + if (loop_count % 64 == 0 && kill(shell_pgid, 0) < 0 && errno == ESRCH) { + we_think_we_are_orphaned = true; } if (!we_think_we_are_orphaned && loop_count % 128 == 0) { @@ -1646,12 +1643,10 @@ static void reader_interactive_init() { // Put ourselves in our own process group. shell_pgid = getpid(); - if (getpgrp() != shell_pgid) { - if (setpgid(shell_pgid, shell_pgid) < 0) { - debug(1, _(L"Couldn't put the shell in its own process group")); - wperror(L"setpgid"); - exit_without_destructors(1); - } + if (getpgrp() != shell_pgid && setpgid(shell_pgid, shell_pgid) < 0) { + debug(1, _(L"Couldn't put the shell in its own process group")); + wperror(L"setpgid"); + exit_without_destructors(1); } // Grab control of the terminal. @@ -2438,42 +2433,40 @@ const wchar_t *reader_readline(int nchars) { is_interactive_read = was_interactive_read; // fprintf(stderr, "C: %lx\n", (long)c); - if (((!fish_reserved_codepoint(c))) && (c > 31) && (c != 127)) { - if (can_read(0)) { - wchar_t arr[READAHEAD_MAX + 1]; - size_t i; - size_t limit = 0 < nchars ? std::min((size_t)nchars - data->command_line.size(), - (size_t)READAHEAD_MAX) - : READAHEAD_MAX; + if (((!fish_reserved_codepoint(c))) && (c > 31) && (c != 127) && can_read(0)) { + wchar_t arr[READAHEAD_MAX + 1]; + size_t i; + size_t limit = 0 < nchars ? std::min((size_t)nchars - data->command_line.size(), + (size_t)READAHEAD_MAX) + : READAHEAD_MAX; - memset(arr, 0, sizeof(arr)); - arr[0] = c; + memset(arr, 0, sizeof(arr)); + arr[0] = c; - for (i = 1; i < limit; ++i) { - if (!can_read(0)) { - c = 0; - break; - } - // Only allow commands on the first key; otherwise, we might have data we - // need to insert on the commandline that the commmand might need to be able - // to see. - c = input_readch(false); - if ((!fish_reserved_codepoint(c)) && (c > 31) && (c != 127)) { - arr[i] = c; - c = 0; - } else - break; + for (i = 1; i < limit; ++i) { + if (!can_read(0)) { + c = 0; + break; } - - editable_line_t *el = data->active_edit_line(); - insert_string(el, arr, true); - - // End paging upon inserting into the normal command line. - if (el == &data->command_line) { - clear_pager(); - } - last_char = c; + // Only allow commands on the first key; otherwise, we might have data we + // need to insert on the commandline that the commmand might need to be able + // to see. + c = input_readch(false); + if (!fish_reserved_codepoint(c) && c > 31 && c != 127) { + arr[i] = c; + c = 0; + } else + break; } + + editable_line_t *el = data->active_edit_line(); + insert_string(el, arr, true); + + // End paging upon inserting into the normal command line. + if (el == &data->command_line) { + clear_pager(); + } + last_char = c; } if (c != 0) break; diff --git a/src/util.cpp b/src/util.cpp index b8f868094..7caba32bc 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -58,12 +58,10 @@ int wcsfilecmp(const wchar_t *a, const wchar_t *b) { int res = wcsfilecmp(a + 1, b + 1); - if (abs(res) < 2) { - // No primary difference in rest of string. Use secondary difference on this element if - // found. - if (secondary_diff) { - return secondary_diff > 0 ? 1 : -1; - } + // If no primary difference in rest of string use secondary difference on this element if + // found. + if (abs(res) < 2 && secondary_diff) { + return secondary_diff > 0 ? 1 : -1; } return res; diff --git a/src/wildcard.cpp b/src/wildcard.cpp index 50468c706..f2ef9fed7 100644 --- a/src/wildcard.cpp +++ b/src/wildcard.cpp @@ -328,14 +328,12 @@ static wcstring file_get_desc(const wcstring &filename, int lstat_res, const str if (S_ISDIR(buf.st_mode)) { return COMPLETE_DIRECTORY_SYMLINK_DESC; } - if (buf.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) { - if (waccess(filename, X_OK) == 0) { - // Weird group permissions and other such issues make it non-trivial to - // find out if we can actually execute a file using the result from - // stat. It is much safer to use the access function, since it tells us - // exactly what we want to know. - return COMPLETE_EXEC_LINK_DESC; - } + if (buf.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH) && waccess(filename, X_OK) == 0) { + // Weird group permissions and other such issues make it non-trivial to + // find out if we can actually execute a file using the result from + // stat. It is much safer to use the access function, since it tells us + // exactly what we want to know. + return COMPLETE_EXEC_LINK_DESC; } return COMPLETE_SYMLINK_DESC; @@ -364,14 +362,12 @@ static wcstring file_get_desc(const wcstring &filename, int lstat_res, const str } else if (S_ISDIR(buf.st_mode)) { return COMPLETE_DIRECTORY_DESC; } else { - if (buf.st_mode & (S_IXUSR | S_IXGRP | S_IXGRP)) { - if (waccess(filename, X_OK) == 0) { - // Weird group permissions and other such issues make it non-trivial to find out - // if we can actually execute a file using the result from stat. It is much - // safer to use the access function, since it tells us exactly what we want to - // know. - return COMPLETE_EXEC_DESC; - } + if (buf.st_mode & (S_IXUSR | S_IXGRP | S_IXGRP) && waccess(filename, X_OK) == 0) { + // Weird group permissions and other such issues make it non-trivial to find out + // if we can actually execute a file using the result from stat. It is much + // safer to use the access function, since it tells us exactly what we want to + // know. + return COMPLETE_EXEC_DESC; } } } @@ -415,15 +411,14 @@ static bool wildcard_test_flags_then_complete(const wcstring &filepath, const wc const bool is_directory = stat_res == 0 && S_ISDIR(stat_buf.st_mode); const bool is_executable = stat_res == 0 && S_ISREG(stat_buf.st_mode); - if (expand_flags & DIRECTORIES_ONLY) { - if (!is_directory) { - return false; - } + const bool need_directory = expand_flags & DIRECTORIES_ONLY; + if (need_directory && !is_directory) { + return false; } - if (expand_flags & EXECUTABLES_ONLY) { - if (!is_executable || waccess(filepath, X_OK) != 0) { - return false; - } + + const bool executables_only = expand_flags & EXECUTABLES_ONLY; + if (executables_only && (!is_executable || waccess(filepath, X_OK) != 0)) { + return false; } // Compute the description.