From 40c90bca9f7e2952bd64faebceb53538f80805a7 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Wed, 7 Sep 2016 21:58:04 +0100 Subject: [PATCH] tidying --- src/src/dbstuff.h | 2 +- src/src/dmarc.c | 2 +- src/src/exim.c | 40 +++++++------------- src/src/log.c | 21 +++-------- src/src/malware.c | 4 +- src/src/mime.c | 10 ++--- src/src/pdkim/pdkim.c | 4 +- src/src/queue.c | 87 +++++++++++++++++++------------------------ src/src/smtp_in.c | 10 ++--- src/src/store.c | 3 +- src/src/tls-gnu.c | 13 +++---- src/src/transport.c | 6 +-- 12 files changed, 81 insertions(+), 121 deletions(-) diff --git a/src/src/dbstuff.h b/src/src/dbstuff.h index ce81f1eb4..93c715ac2 100644 --- a/src/src/dbstuff.h +++ b/src/src/dbstuff.h @@ -64,7 +64,7 @@ tdb_traverse to be called) */ /* EXIM_DBCREATE_CURSOR - initialize for scanning operation */ #define EXIM_DBCREATE_CURSOR(db, cursor) { \ - *(cursor) = malloc(sizeof(TDB_DATA)); (*(cursor))->dptr = NULL; } + *(cursor) = store_malloc(sizeof(TDB_DATA)); (*(cursor))->dptr = NULL; } /* EXIM_DBSCAN - This is complicated because we have to free the last datum free() must not die when passed NULL */ diff --git a/src/src/dmarc.c b/src/src/dmarc.c index 2fdc9eda3..c005d4ab9 100644 --- a/src/src/dmarc.c +++ b/src/src/dmarc.c @@ -57,7 +57,7 @@ static dmarc_exim_p dmarc_policy_description[] = { static error_block * add_to_eblock(error_block *eblock, uschar *t1, uschar *t2) { -error_block *eb = malloc(sizeof(error_block)); +error_block *eb = store_malloc(sizeof(error_block)); if (eblock == NULL) eblock = eb; else diff --git a/src/src/exim.c b/src/src/exim.c index 1ad76dea2..69d2edb8b 100644 --- a/src/src/exim.c +++ b/src/src/exim.c @@ -1653,8 +1653,7 @@ os_non_restarting_signal(SIGALRM, sigalrm_handler); /* Ensure we have a buffer for constructing log entries. Use malloc directly, because store_malloc writes a log entry on failure. */ -log_buffer = (uschar *)malloc(LOG_BUFFER_SIZE); -if (log_buffer == NULL) +if (!(log_buffer = US malloc(LOG_BUFFER_SIZE))) { fprintf(stderr, "exim: failed to get store for log buffer\n"); exit(EXIT_FAILURE); @@ -3938,7 +3937,6 @@ if (Ustrlen(syslog_processname) > 32) "syslog_processname is longer than 32 chars: aborting"); if (log_oneline) - { if (admin_user) { log_write(0, LOG_MAIN, "%s", log_oneline); @@ -3946,7 +3944,6 @@ if (log_oneline) } else return EXIT_FAILURE; - } /* In some operating systems, the environment variable TMPDIR controls where temporary files are created; Exim doesn't use these (apart from when delivering @@ -3960,17 +3957,14 @@ EXIM_TMPDIR by the build scripts. #ifdef EXIM_TMPDIR { uschar **p; - if (environ) for (p = USS environ; *p != NULL; p++) - { - if (Ustrncmp(*p, "TMPDIR=", 7) == 0 && - Ustrcmp(*p+7, EXIM_TMPDIR) != 0) + if (environ) for (p = USS environ; *p; p++) + if (Ustrncmp(*p, "TMPDIR=", 7) == 0 && Ustrcmp(*p+7, EXIM_TMPDIR) != 0) { - uschar *newp = malloc(Ustrlen(EXIM_TMPDIR) + 8); + uschar * newp = store_malloc(Ustrlen(EXIM_TMPDIR) + 8); sprintf(CS newp, "TMPDIR=%s", EXIM_TMPDIR); *p = newp; DEBUG(D_any) debug_printf("reset TMPDIR=%s in environment\n", EXIM_TMPDIR); } - } } #endif @@ -3984,33 +3978,25 @@ about this earlier - but hopefully nothing will normally be logged earlier than this. We have to make a new environment if TZ is wrong, but don't bother if timestamps_utc is set, because then all times are in UTC anyway. */ -if (timezone_string != NULL && strcmpic(timezone_string, US"UTC") == 0) - { +if (timezone_string && strcmpic(timezone_string, US"UTC") == 0) timestamps_utc = TRUE; - } else { uschar *envtz = US getenv("TZ"); - if ((envtz == NULL && timezone_string != NULL) || - (envtz != NULL && - (timezone_string == NULL || - Ustrcmp(timezone_string, envtz) != 0))) + if (envtz ? !timezone_string || Ustrcmp(timezone_string, envtz) != 0 : timezone_string) { uschar **p = USS environ; uschar **new; uschar **newp; int count = 0; - if (environ) while (*p++ != NULL) count++; - if (envtz == NULL) count++; - newp = new = malloc(sizeof(uschar *) * (count + 1)); - if (environ) for (p = USS environ; *p != NULL; p++) - { - if (Ustrncmp(*p, "TZ=", 3) == 0) continue; - *newp++ = *p; - } - if (timezone_string != NULL) + if (environ) while (*p++) count++; + if (!envtz) count++; + newp = new = store_malloc(sizeof(uschar *) * (count + 1)); + if (environ) for (p = USS environ; *p; p++) + if (Ustrncmp(*p, "TZ=", 3) != 0) *newp++ = *p; + if (timezone_string) { - *newp = malloc(Ustrlen(timezone_string) + 4); + *newp = store_malloc(Ustrlen(timezone_string) + 4); sprintf(CS *newp++, "TZ=%s", timezone_string); } *newp = NULL; diff --git a/src/src/log.c b/src/src/log.c index b01a179c0..860b8b0a2 100644 --- a/src/src/log.c +++ b/src/src/log.c @@ -490,12 +490,9 @@ log. If possible, save a copy of the original line that was being logged. If we are recursing (can't open the panic log either), the pointer will already be set. */ -if (panic_save_buffer == NULL) - { - panic_save_buffer = (uschar *)malloc(LOG_BUFFER_SIZE); - if (panic_save_buffer != NULL) +if (!panic_save_buffer) + if ((panic_save_buffer = US malloc(LOG_BUFFER_SIZE))) memcpy(panic_save_buffer, log_buffer, LOG_BUFFER_SIZE); - } log_write(0, LOG_PANIC_DIE, "Cannot open %s log file \"%s\": %s: " "euid=%d egid=%d", log_names[type], buffer, strerror(errno), euid, getegid()); @@ -575,12 +572,9 @@ log_write_failed(uschar *name, int length, int rc) { int save_errno = errno; -if (panic_save_buffer == NULL) - { - panic_save_buffer = (uschar *)malloc(LOG_BUFFER_SIZE); - if (panic_save_buffer != NULL) +if (!panic_save_buffer) + if ((panic_save_buffer = US malloc(LOG_BUFFER_SIZE))) memcpy(panic_save_buffer, log_buffer, LOG_BUFFER_SIZE); - } log_write(0, LOG_PANIC_DIE, "failed to write to %s: length=%d result=%d " "errno=%d (%s)", name, length, rc, save_errno, @@ -736,15 +730,12 @@ if (panic_recurseflag) /* Ensure we have a buffer (see comment above); this should never be obeyed when running Exim proper, only when running utilities. */ -if (log_buffer == NULL) - { - log_buffer = (uschar *)malloc(LOG_BUFFER_SIZE); - if (log_buffer == NULL) +if (!log_buffer) + if (!(log_buffer = US malloc(LOG_BUFFER_SIZE))) { fprintf(stderr, "exim: failed to get store for log buffer\n"); exim_exit(EXIT_FAILURE); } - } /* If we haven't already done so, inspect the setting of log_file_path to determine whether to log to files and/or to syslog. Bits in logging_mode diff --git a/src/src/malware.c b/src/src/malware.c index a5944cafb..b4a7f7094 100644 --- a/src/src/malware.c +++ b/src/src/malware.c @@ -630,7 +630,7 @@ if (!malware_ok) sock); } - if (!(drweb_fbuf = (uschar *) malloc (fsize_uint))) + if (!(drweb_fbuf = US malloc(fsize_uint))) { (void)close(drweb_fd); return m_errlog_defer_3(scanent, NULL, @@ -1486,7 +1486,7 @@ if (!malware_ok) } lseek(clam_fd, 0, SEEK_SET); - if (!(clamav_fbuf = (uschar *) malloc (fsize_uint))) + if (!(clamav_fbuf = US malloc(fsize_uint))) { CLOSE_SOCKDATA; (void)close(clam_fd); return m_errlog_defer_3(scanent, NULL, diff --git a/src/src/mime.c b/src/src/mime.c index 0339295f3..c924f2bc3 100644 --- a/src/src/mime.c +++ b/src/src/mime.c @@ -192,13 +192,11 @@ static FILE * mime_get_decode_file(uschar *pname, uschar *fname) { FILE *f = NULL; -uschar *filename; - -filename = (uschar *)malloc(2048); +uschar *filename = NULL; if (pname && fname) { - (void)string_format(filename, 2048, "%s/%s", pname, fname); + filename = string_sprintf("%s/%s", pname, fname); f = modefopen(filename,"wb+",SPOOL_MODE); } else if (!pname) @@ -212,8 +210,7 @@ else if (!fname) do { struct stat mystat; - (void)string_format(filename, 2048, - "%s/%s-%05u", pname, message_id, file_nr++); + filename = string_sprintf("%s/%s-%05u", pname, message_id, file_nr++); /* security break */ if (file_nr >= 1024) break; @@ -224,6 +221,7 @@ else if (!fname) } /* set expansion variable */ +/*XXX ? not set if !pname ? */ mime_decoded_filename = filename; return f; diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c index 15f868e63..fd3c6bc2b 100644 --- a/src/src/pdkim/pdkim.c +++ b/src/src/pdkim/pdkim.c @@ -728,7 +728,7 @@ while (sig) we don't know what allocations the SHA routines might do, not safe to use store_get()/store_reset(). */ - relaxed_data = malloc(len+1); + relaxed_data = store_malloc(len+1); for (p = data; *p; p++) { @@ -772,7 +772,7 @@ while (sig) sig = sig->next; } -if (relaxed_data) free(relaxed_data); +if (relaxed_data) store_free(relaxed_data); return PDKIM_OK; } diff --git a/src/src/queue.c b/src/src/queue.c index 7648f47ca..16e18efc5 100644 --- a/src/src/queue.c +++ b/src/src/queue.c @@ -76,8 +76,7 @@ merge_queue_lists(queue_filename *a, queue_filename *b) queue_filename *first = NULL; queue_filename **append = &first; -while (a != NULL && b != NULL) - { +while (a && b) if (Ustrcmp(a->text, b->text) < 0) { *append = a; @@ -90,9 +89,8 @@ while (a != NULL && b != NULL) append= &b->next; b = b->next; } - } -*append=((a != NULL)? a : b); +*append = a ? a : b; return first; } @@ -161,8 +159,11 @@ according to the bits of the flags variable. Get a collection of bits from the current time. Use the bottom 16 and just keep re-using them if necessary. When not randomizing, initialize the sublists for the bottom-up merge sort. */ -if (randomize) resetflags = time(NULL) & 0xFFFF; - else for (i = 0; i < LOG2_MAXNODES; i++) root[i] = NULL; +if (randomize) + resetflags = time(NULL) & 0xFFFF; +else + for (i = 0; i < LOG2_MAXNODES; i++) + root[i] = NULL; /* If processing the full queue, or just the top-level, start at the base directory, and initialize the first subdirectory name (as none). Otherwise, @@ -174,7 +175,8 @@ if (subdiroffset <= 0) subdirs[0] = 0; *subcount = 0; } -else i = subdiroffset; +else + i = subdiroffset; /* Set up prototype for the directory name. */ @@ -204,7 +206,7 @@ for (; i <= *subcount; i++) /* Now scan the directory. */ - while ((ent = readdir(dd)) != NULL) + while ((ent = readdir(dd))) { uschar *name = US ent->d_name; int len = Ustrlen(name); @@ -240,15 +242,15 @@ for (; i <= *subcount; i++) to store the number with each item. */ if (randomize) - { - if (yield == NULL) + if (!yield) { next->next = NULL; yield = last = next; } else { - if (flags == 0) flags = resetflags; + if (flags == 0) + flags = resetflags; if ((flags & 1) == 0) { next->next = yield; @@ -262,7 +264,6 @@ for (; i <= *subcount; i++) } flags = flags >> 1; } - } /* Otherwise do a bottom-up merge sort based on the name. */ @@ -271,8 +272,7 @@ for (; i <= *subcount; i++) int j; next->next = NULL; for (j = 0; j < LOG2_MAXNODES; j++) - { - if (root[j] != NULL) + if (root[j]) { next = merge_queue_lists(next, root[j]); root[j] = (j == LOG2_MAXNODES - 1)? next : NULL; @@ -282,7 +282,6 @@ for (; i <= *subcount; i++) root[j] = next; break; } - } } } } @@ -314,10 +313,8 @@ for (; i <= *subcount; i++) /* If we have just scanned the base directory, and subdiroffset is 0, we do not want to continue scanning the sub-directories. */ - else - { - if (subdiroffset == 0) break; - } + else if (subdiroffset == 0) + break; } /* Loop for multiple subdirectories */ /* When using a bottom-up merge sort, do the final merging of the sublists. @@ -478,7 +475,7 @@ for (i = (queue_run_in_order? -1 : 0); } for (f = queue_get_spool_list(i, subdirs, &subcount, !queue_run_in_order); - f != NULL; + f; f = f->next) { pid_t pid; @@ -491,9 +488,7 @@ for (i = (queue_run_in_order? -1 : 0); check that the load average is low enough to permit deliveries. */ if (!queue_run_force && deliver_queue_load_max >= 0) - { - load_average = os_getloadavg(); - if (load_average > deliver_queue_load_max) + if ((load_average = os_getloadavg()) > deliver_queue_load_max) { log_write(L_queue_run, LOG_MAIN, "Abandon queue run: %s (load %.2f, max %.2f)", log_detail, @@ -503,18 +498,15 @@ for (i = (queue_run_in_order? -1 : 0); break; } else - { DEBUG(D_load) debug_printf("load average = %.2f max = %.2f\n", (double)load_average/1000.0, (double)deliver_queue_load_max/1000.0); - } - } /* Skip this message unless it's within the ID limits */ - if (stop_id != NULL && Ustrncmp(f->text, stop_id, MESSAGE_ID_LENGTH) > 0) + if (stop_id && Ustrncmp(f->text, stop_id, MESSAGE_ID_LENGTH) > 0) continue; - if (start_id != NULL && Ustrncmp(f->text, start_id, MESSAGE_ID_LENGTH) < 0) + if (start_id && Ustrncmp(f->text, start_id, MESSAGE_ID_LENGTH) < 0) continue; /* Check that the message still exists */ @@ -529,7 +521,7 @@ for (i = (queue_run_in_order? -1 : 0); delivering, but it's cheaper than forking a delivery process for each message when many are not going to be delivered. */ - if (deliver_selectstring != NULL || deliver_selectstring_sender != NULL || + if (deliver_selectstring || deliver_selectstring_sender || queue_run_first_delivery) { BOOL wanted = TRUE; @@ -562,19 +554,20 @@ for (i = (queue_run_in_order? -1 : 0); wanted = FALSE; } - /* Check for a matching address if deliver_selectstring[_sender} is set. + /* Check for a matching address if deliver_selectstring[_sender] is set. If so, we do a fully delivery - don't want to omit other addresses since their routing might trigger re-writing etc. */ /* Sender matching */ - else if (deliver_selectstring_sender != NULL && - !(deliver_selectstring_sender_regex? - (pcre_exec(selectstring_regex_sender, NULL, CS sender_address, - Ustrlen(sender_address), 0, PCRE_EOPT, NULL, 0) >= 0) - : - (strstric(sender_address, deliver_selectstring_sender, FALSE) - != NULL))) + else if ( deliver_selectstring_sender + && !(deliver_selectstring_sender_regex + ? (pcre_exec(selectstring_regex_sender, NULL, + CS sender_address, Ustrlen(sender_address), 0, PCRE_EOPT, + NULL, 0) >= 0) + : (strstric(sender_address, deliver_selectstring_sender, FALSE) + != NULL) + ) ) { DEBUG(D_queue_run) debug_printf("%s: sender address did not match %s\n", f->text, deliver_selectstring_sender); @@ -583,19 +576,19 @@ for (i = (queue_run_in_order? -1 : 0); /* Recipient matching */ - else if (deliver_selectstring != NULL) + else if (deliver_selectstring) { int i; for (i = 0; i < recipients_count; i++) { uschar *address = recipients_list[i].address; - if ((deliver_selectstring_regex? - (pcre_exec(selectstring_regex, NULL, CS address, - Ustrlen(address), 0, PCRE_EOPT, NULL, 0) >= 0) - : - (strstric(address, deliver_selectstring, FALSE) != NULL)) - && - tree_search(tree_nonrecipients, address) == NULL) + if ( (deliver_selectstring_regex + ? (pcre_exec(selectstring_regex, NULL, CS address, + Ustrlen(address), 0, PCRE_EOPT, NULL, 0) >= 0) + : (strstric(address, deliver_selectstring, FALSE) != NULL) + ) + && tree_search(tree_nonrecipients, address) == NULL + ) break; } @@ -624,10 +617,8 @@ for (i = (queue_run_in_order? -1 : 0); pretty cheap. */ if (pipe(pfd) < 0) - { log_write(0, LOG_MAIN|LOG_PANIC_DIE, "failed to create pipe in queue " "runner process %d: %s", queue_run_pid, strerror(errno)); - } queue_run_pipe = pfd[pipe_write]; /* To ensure it gets passed on. */ /* Make sure it isn't stdin. This seems unlikely, but just to be on the @@ -681,11 +672,9 @@ for (i = (queue_run_in_order? -1 : 0); /* If the process crashed, tell somebody */ else if ((status & 0x00ff) != 0) - { log_write(0, LOG_MAIN|LOG_PANIC, "queue run: process %d crashed with signal %d while delivering %s", (int)pid, status & 0x00ff, f->text); - } /* Before continuing, wait till the pipe gets closed at the far end. This tells us that any children created by the delivery to re-use any SMTP diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 387affaf3..3b631ea10 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -2049,10 +2049,10 @@ acl_var_c = NULL; /* Allow for trailing 0 in the command and data buffers. */ -smtp_cmd_buffer = (uschar *)malloc(2*smtp_cmd_buffer_size + 2); -if (smtp_cmd_buffer == NULL) +if (!(smtp_cmd_buffer = US malloc(2*smtp_cmd_buffer_size + 2))) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "malloc() failed for SMTP command buffer"); + smtp_cmd_buffer[0] = 0; smtp_data_buffer = smtp_cmd_buffer + smtp_cmd_buffer_size + 1; @@ -2061,7 +2061,7 @@ command line by a trusted caller. */ if (smtp_batched_input) { - if (received_protocol == NULL) received_protocol = US"local-bsmtp"; + if (!received_protocol) received_protocol = US"local-bsmtp"; } /* For non-batched SMTP input, the protocol setting is forced here. It will be @@ -2074,9 +2074,9 @@ else /* Set up the buffer for inputting using direct read() calls, and arrange to call the local functions instead of the standard C ones. */ -smtp_inbuffer = (uschar *)malloc(in_buffer_size); -if (smtp_inbuffer == NULL) +if (!(smtp_inbuffer = (uschar *)malloc(in_buffer_size))) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "malloc() failed for SMTP input buffer"); + receive_getc = smtp_getc; receive_get_cache = smtp_get_cache; receive_ungetc = smtp_ungetc; diff --git a/src/src/store.c b/src/src/store.c index 88b1fd88f..b1a47799b 100644 --- a/src/src/store.c +++ b/src/src/store.c @@ -497,9 +497,8 @@ store_malloc_3(int size, const char *filename, int linenumber) void *yield; if (size < 16) size = 16; -yield = malloc((size_t)size); -if (yield == NULL) +if (!(yield = malloc((size_t)size))) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "failed to malloc %d bytes of memory: " "called from line %d of %s", size, linenumber, filename); diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c index c9dc4d9be..a5a680fd2 100644 --- a/src/src/tls-gnu.c +++ b/src/src/tls-gnu.c @@ -566,8 +566,7 @@ if (fd >= 0) (void)close(fd); return tls_error(US"TLS cache not a file", NULL, NULL); } - fp = fdopen(fd, "rb"); - if (!fp) + if (!(fp = fdopen(fd, "rb"))) { saved_errno = errno; (void)close(fd); @@ -576,14 +575,12 @@ if (fd >= 0) } m.size = statbuf.st_size; - m.data = malloc(m.size); - if (m.data == NULL) + if (!(m.data = malloc(m.size))) { fclose(fp); return tls_error(US"malloc failed", strerror(errno), NULL); } - sz = fread(m.data, m.size, 1, fp); - if (!sz) + if (!(sz = fread(m.data, m.size, 1, fp))) { saved_errno = errno; fclose(fp); @@ -665,9 +662,9 @@ if (rc < 0) if (rc != GNUTLS_E_SHORT_MEMORY_BUFFER) exim_gnutls_err_check(US"gnutls_dh_params_export_pkcs3(NULL) sizing"); m.size = sz; - m.data = malloc(m.size); - if (m.data == NULL) + if (!(m.data = malloc(m.size))) return tls_error(US"memory allocation failed", strerror(errno), NULL); + /* this will return a size 1 less than the allocation size above */ rc = gnutls_dh_params_export_pkcs3(dh_server_params, GNUTLS_X509_FMT_PEM, m.data, &sz); diff --git a/src/src/transport.c b/src/src/transport.c index c998404b2..efc30be59 100644 --- a/src/src/transport.c +++ b/src/src/transport.c @@ -1758,7 +1758,7 @@ while (1) /* create an array to read entire message queue into memory for processing */ - msgq = (msgq_t*) malloc(sizeof(msgq_t) * host_record->count); + msgq = store_malloc(sizeof(msgq_t) * host_record->count); msgq_count = host_record->count; msgq_actual = msgq_count; @@ -1866,7 +1866,7 @@ test but the code should work */ if (bFound) /* Usual exit from main loop */ { - free (msgq); + store_free (msgq); break; } @@ -1892,7 +1892,7 @@ test but the code should work */ return FALSE; } - free(msgq); + store_free(msgq); } /* we need to process a continuation record */ /* Control gets here when an existing message has been encountered; its -- 2.25.1