From 2b60ac102164f379dff0f26a42f9bb14c9ce94ad Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Mon, 25 May 2020 23:08:53 +0100 Subject: [PATCH 1/1] tidying --- src/src/acl.c | 30 ++++--- src/src/deliver.c | 20 +++-- src/src/exim.c | 134 ++++++++++++++------------------ src/src/transports/appendfile.c | 17 ++-- src/src/verify.c | 8 +- 5 files changed, 95 insertions(+), 114 deletions(-) diff --git a/src/src/acl.c b/src/src/acl.c index c1d60bbd9..24716f0d1 100644 --- a/src/src/acl.c +++ b/src/src/acl.c @@ -1515,7 +1515,7 @@ static verify_type_t verify_type_list[] = { { US"not_blind", VERIFY_NOT_BLIND, ACL_BIT_DATA | ACL_BIT_NOTSMTP, FALSE, 0 }, { US"header_sender", VERIFY_HDR_SNDR, ACL_BIT_DATA | ACL_BIT_NOTSMTP, FALSE, 0 }, { US"sender", VERIFY_SNDR, ACL_BIT_MAIL | ACL_BIT_RCPT - |ACL_BIT_PREDATA | ACL_BIT_DATA | ACL_BIT_NOTSMTP, + | ACL_BIT_PREDATA | ACL_BIT_DATA | ACL_BIT_NOTSMTP, FALSE, 6 }, { US"recipient", VERIFY_RCPT, ACL_BIT_RCPT, FALSE, 0 }, { US"header_names_ascii", VERIFY_HDR_NAMES_ASCII, ACL_BIT_DATA | ACL_BIT_NOTSMTP, TRUE, 0 }, @@ -1746,7 +1746,7 @@ switch(vp->value) in place of the actual sender (rare special-case requirement). */ { uschar *s = ss + 6; - if (*s == 0) + if (!*s) verify_sender_address = sender_address; else { @@ -1792,19 +1792,16 @@ while ((ss = string_nextinlist(&list, &sep, big_buffer, big_buffer_size))) else if (strncmpic(ss, US"callout", 7) == 0) { callout = CALLOUT_TIMEOUT_DEFAULT; - ss += 7; - if (*ss != 0) + if (*(ss += 7)) { while (isspace(*ss)) ss++; if (*ss++ == '=') { const uschar * sublist = ss; int optsep = ','; - uschar buffer[256]; - uschar * opt; while (isspace(*sublist)) sublist++; - while ((opt = string_nextinlist(&sublist, &optsep, buffer, sizeof(buffer)))) + for (uschar * opt; opt = string_nextinlist(&sublist, &optsep, NULL, 0); ) { callout_opt_t * op; double period = 1.0F; @@ -1928,8 +1925,8 @@ else if (verify_sender_address) } sender_vaddr = verify_checked_sender(verify_sender_address); - if (sender_vaddr != NULL && /* Previously checked */ - callout <= 0) /* No callout needed this time */ + if ( sender_vaddr /* Previously checked */ + && callout <= 0) /* No callout needed this time */ { /* If the "routed" flag is set, it means that routing worked before, so this check can give OK (the saved return code value, if set, belongs to a @@ -1996,14 +1993,12 @@ else if (verify_sender_address) *basic_errno = sender_vaddr->basic_errno; else DEBUG(D_acl) - { if (Ustrcmp(sender_vaddr->address, verify_sender_address) != 0) debug_printf_indent("sender %s verified ok as %s\n", verify_sender_address, sender_vaddr->address); else debug_printf_indent("sender %s verified ok\n", verify_sender_address); - } } else rc = OK; /* Null sender */ @@ -2047,8 +2042,7 @@ else *basic_errno = addr2.basic_errno; *log_msgptr = addr2.message; - *user_msgptr = (addr2.user_message != NULL)? - addr2.user_message : addr2.message; + *user_msgptr = addr2.user_message ? addr2.user_message : addr2.message; /* Allow details for temporary error if the address is so flagged. */ if (testflag((&addr2), af_pass_message)) f.acl_temp_details = TRUE; @@ -2059,8 +2053,10 @@ else /* We have a result from the relevant test. Handle defer overrides first. */ -if (rc == DEFER && (defer_ok || - (callout_defer_ok && *basic_errno == ERRNO_CALLOUTDEFER))) +if ( rc == DEFER + && ( defer_ok + || callout_defer_ok && *basic_errno == ERRNO_CALLOUTDEFER + ) ) { HDEBUG(D_acl) debug_printf_indent("verify defer overridden by %s\n", defer_ok? "defer_ok" : "callout_defer_ok"); @@ -2070,7 +2066,7 @@ if (rc == DEFER && (defer_ok || /* If we've failed a sender, set up a recipient message, and point sender_verified_failed to the address item that actually failed. */ -if (rc != OK && verify_sender_address != NULL) +if (rc != OK && verify_sender_address) { if (rc != DEFER) *log_msgptr = *user_msgptr = US"Sender verify failed"; @@ -2089,7 +2085,7 @@ if (rc != OK && verify_sender_address != NULL) /* Verifying an address messes up the values of $domain and $local_part, so reset them before returning if this is a RCPT ACL. */ -if (addr != NULL) +if (addr) { deliver_domain = addr->domain; deliver_localpart = addr->local_part; diff --git a/src/src/deliver.c b/src/src/deliver.c index 67d711b7e..acb87bca6 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -2152,18 +2152,16 @@ else if (tp->return_path) { - uschar *new_return_path = expand_string(tp->return_path); - if (!new_return_path) - { - if (!f.expand_string_forcedfail) - { - common_error(TRUE, addr, ERRNO_EXPANDFAIL, - US"Failed to expand return path \"%s\" in %s transport: %s", - tp->return_path, tp->name, expand_string_message); - return; - } + uschar * new_return_path = expand_string(tp->return_path); + if (new_return_path) + return_path = new_return_path; + else if (!f.expand_string_forcedfail) + { + common_error(TRUE, addr, ERRNO_EXPANDFAIL, + US"Failed to expand return path \"%s\" in %s transport: %s", + tp->return_path, tp->name, expand_string_message); + return; } - else return_path = new_return_path; } /* For local deliveries, one at a time, the value used for logging can just be diff --git a/src/src/exim.c b/src/src/exim.c index da4341421..905ef0a70 100644 --- a/src/src/exim.c +++ b/src/src/exim.c @@ -3504,54 +3504,40 @@ END_ARG: if (usage_wanted) exim_usage(called_as); /* Arguments have been processed. Check for incompatibilities. */ -if (( - (smtp_input || extract_recipients || recipients_arg < argc) && - (f.daemon_listen || queue_interval >= 0 || bi_option || - test_retry_arg >= 0 || test_rewrite_arg >= 0 || - filter_test != FTEST_NONE || (msg_action_arg > 0 && !one_msg_action)) - ) || - ( - msg_action_arg > 0 && - (f.daemon_listen || queue_interval > 0 || list_options || - (checking && msg_action != MSG_LOAD) || - bi_option || test_retry_arg >= 0 || test_rewrite_arg >= 0) - ) || - ( - (f.daemon_listen || queue_interval > 0) && - (sender_address != NULL || list_options || list_queue || checking || - bi_option) - ) || - ( - f.daemon_listen && queue_interval == 0 - ) || - ( - f.inetd_wait_mode && queue_interval >= 0 - ) || - ( - list_options && - (checking || smtp_input || extract_recipients || - filter_test != FTEST_NONE || bi_option) - ) || - ( - verify_address_mode && - (f.address_test_mode || smtp_input || extract_recipients || - filter_test != FTEST_NONE || bi_option) - ) || - ( - f.address_test_mode && (smtp_input || extract_recipients || - filter_test != FTEST_NONE || bi_option) - ) || - ( - smtp_input && (sender_address != NULL || filter_test != FTEST_NONE || - extract_recipients) - ) || - ( - deliver_selectstring != NULL && queue_interval < 0 - ) || - ( - msg_action == MSG_LOAD && - (!expansion_test || expansion_test_message != NULL) - ) +if ( ( (smtp_input || extract_recipients || recipients_arg < argc) + && ( f.daemon_listen || queue_interval >= 0 || bi_option + || test_retry_arg >= 0 || test_rewrite_arg >= 0 + || filter_test != FTEST_NONE + || msg_action_arg > 0 && !one_msg_action + ) ) + || ( msg_action_arg > 0 + && ( f.daemon_listen || queue_interval > 0 || list_options + || checking && msg_action != MSG_LOAD + || bi_option || test_retry_arg >= 0 || test_rewrite_arg >= 0 + ) ) + || ( (f.daemon_listen || queue_interval > 0) + && ( sender_address || list_options || list_queue || checking + || bi_option + ) ) + || f.daemon_listen && queue_interval == 0 + || f.inetd_wait_mode && queue_interval >= 0 + || ( list_options + && ( checking || smtp_input || extract_recipients + || filter_test != FTEST_NONE || bi_option + ) ) + || ( verify_address_mode + && ( f.address_test_mode || smtp_input || extract_recipients + || filter_test != FTEST_NONE || bi_option + ) ) + || ( f.address_test_mode + && ( smtp_input || extract_recipients || filter_test != FTEST_NONE + || bi_option + ) ) + || ( smtp_input + && (sender_address || filter_test != FTEST_NONE || extract_recipients) + ) + || deliver_selectstring && queue_interval < 0 + || msg_action == MSG_LOAD && (!expansion_test || expansion_test_message) ) exim_fail("exim: incompatible command-line options or arguments\n"); @@ -4172,7 +4158,7 @@ if (!f.admin_user) || queue_name_dest && prod_requires_admin || debugset && !f.running_in_test_harness ) - exim_fail("exim:%s permission denied\n", debugset? " debugging" : ""); + exim_fail("exim:%s permission denied\n", debugset ? " debugging" : ""); } /* If the real user is not root or the exim uid, the argument for passing @@ -4181,11 +4167,13 @@ running with the -N option for any delivery action, unless this call to exim is one that supplied an input message, or we are using a patched exim for regression testing. */ -if (real_uid != root_uid && real_uid != exim_uid && - (continue_hostname != NULL || - (f.dont_deliver && - (queue_interval >= 0 || f.daemon_listen || msg_action_arg > 0) - )) && !f.running_in_test_harness) +if ( real_uid != root_uid && real_uid != exim_uid + && ( continue_hostname + || ( f.dont_deliver + && (queue_interval >= 0 || f.daemon_listen || msg_action_arg > 0) + ) ) + && !f.running_in_test_harness + ) exim_fail("exim: Permission denied\n"); /* If the caller is not trusted, certain arguments are ignored when running for @@ -4207,9 +4195,9 @@ Exim exits if the syntax is bad. */ else { - if (sender_host_address != NULL) + if (sender_host_address) sender_host_port = check_port(sender_host_address); - if (interface_address != NULL) + if (interface_address) interface_port = check_port(interface_address); } @@ -4296,18 +4284,19 @@ retained only for starting the daemon. We always do the initgroups() in this situation (controlled by the TRUE below), in order to be as close as possible to the state Exim usually runs in. */ -if (!unprivileged && /* originally had root AND */ - !removed_privilege && /* still got root AND */ - !f.daemon_listen && /* not starting the daemon */ - queue_interval <= 0 && /* (either kind of daemon) */ - ( /* AND EITHER */ - deliver_drop_privilege || /* requested unprivileged */ - ( /* OR */ - queue_interval < 0 && /* not running the queue */ - (msg_action_arg < 0 || /* and */ - msg_action != MSG_DELIVER) && /* not delivering and */ - (!checking || !f.address_test_mode) /* not address checking */ - ) ) ) +if ( !unprivileged /* originally had root AND */ + && !removed_privilege /* still got root AND */ + && !f.daemon_listen /* not starting the daemon */ + && queue_interval <= 0 /* (either kind of daemon) */ + && ( /* AND EITHER */ + deliver_drop_privilege /* requested unprivileged */ + || ( /* OR */ + queue_interval < 0 /* not running the queue */ + && ( msg_action_arg < 0 /* and */ + || msg_action != MSG_DELIVER /* not delivering */ + ) /* and */ + && (!checking || !f.address_test_mode) /* not address checking */ + ) ) ) exim_setugid(exim_uid, exim_gid, TRUE, US"privilege not needed"); /* When we are retaining a privileged uid, we still change to the exim gid. */ @@ -4336,8 +4325,7 @@ if (malware_test_file) #ifdef WITH_CONTENT_SCAN int result; set_process_info("scanning file for malware"); - result = malware_in_file(malware_test_file); - if (result == FAIL) + if ((result = malware_in_file(malware_test_file)) == FAIL) { printf("No malware found.\n"); exit(EXIT_SUCCESS); @@ -4860,8 +4848,8 @@ if ( !smtp_input && !sender_address sender, or if a sender other than <> is set, override with the originator's login (which will get qualified below), except when checking things. */ - if (sender_address == NULL /* No sender_address set */ - || /* OR */ + if ( !sender_address /* No sender_address set */ + || /* OR */ (sender_address[0] != 0 && /* Non-empty sender address, AND */ !checking)) /* Not running tests, including filter tests */ { @@ -4912,7 +4900,6 @@ if (verify_address_mode || f.address_test_mode) } if (recipients_arg < argc) - { while (recipients_arg < argc) { /* Supplied addresses are tainted since they come from a user */ @@ -4928,7 +4915,6 @@ if (verify_address_mode || f.address_test_mode) while (*++s == ',' || isspace(*s)) ; } } - } else for (;;) { diff --git a/src/src/transports/appendfile.c b/src/src/transports/appendfile.c index f96d00182..f07f3bacb 100644 --- a/src/src/transports/appendfile.c +++ b/src/src/transports/appendfile.c @@ -2287,14 +2287,14 @@ else { uschar *new_check_path = string_copy(check_path); uschar *slash = Ustrrchr(new_check_path, '/'); - if (slash != NULL) + if (slash) { - if (slash[1] == 0) + if (!slash[1]) { *slash = 0; slash = Ustrrchr(new_check_path, '/'); } - if (slash != NULL) + if (slash) { *slash = 0; check_path = new_check_path; @@ -2349,7 +2349,7 @@ else { uschar *s = path + check_path_len; while (*s == '/') s++; - s = (*s == 0) ? US "new" : string_sprintf("%s/new", s); + s = *s ? string_sprintf("%s/new", s) : US"new"; if (pcre_exec(dir_regex, NULL, CS s, Ustrlen(s), 0, 0, NULL, 0) < 0) { disable_quota = TRUE; @@ -2408,10 +2408,11 @@ else count. Note that ob->quota_filecount_value cannot be set without ob->quota_value being set. */ - if (!disable_quota && - (ob->quota_value > 0 || THRESHOLD_CHECK) && - (mailbox_size < 0 || - (mailbox_filecount < 0 && ob->quota_filecount_value > 0))) + if ( !disable_quota + && (ob->quota_value > 0 || THRESHOLD_CHECK) + && ( mailbox_size < 0 + || mailbox_filecount < 0 && ob->quota_filecount_value > 0 + ) ) { off_t size; int filecount = 0; diff --git a/src/src/verify.c b/src/src/verify.c index 76f6c9167..ab513dab3 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -1612,7 +1612,7 @@ address testing (-bt), which is indicated by address_test_mode being set. Arguments: vaddr contains the address to verify; the next field in this block must be NULL - f if not NULL, write the result to this file + fp if not NULL, write the result to this file options various option bits: vopt_fake_sender => this sender verify is not for the real sender (it was verify=sender=xxxx or an address from a @@ -1663,9 +1663,9 @@ BOOL expn = (options & vopt_expn) != 0; BOOL success_on_redirect = (options & vopt_success_on_redirect) != 0; int i; int yield = OK; -int verify_type = expn? v_expn : - f.address_test_mode? v_none : - options & vopt_is_recipient? v_recipient : v_sender; +int verify_type = expn ? v_expn : + f.address_test_mode ? v_none : + options & vopt_is_recipient ? v_recipient : v_sender; address_item *addr_list; address_item *addr_new = NULL; address_item *addr_remote = NULL; -- 2.30.2