From 68907c0fdb1a6ee95cef6550b1333bf34d1a3388 Mon Sep 17 00:00:00 2001 From: Evan Zelkowitz Date: Thu, 30 Jan 2020 11:32:48 -0700 Subject: [PATCH 1/5] Change bitfields to be unsigned explicitly (#6373) Addresses issue #6350 (cherry picked from commit bea9d089c9086859d7bbe9a983b625dfe1b1a388) --- plugins/lua/ts_lua_common.h | 12 ++++++------ plugins/lua/ts_lua_coroutine.h | 2 +- plugins/lua/ts_lua_fetch.h | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/lua/ts_lua_common.h b/plugins/lua/ts_lua_common.h index dba8b7d77e0..628eb09dd68 100644 --- a/plugins/lua/ts_lua_common.h +++ b/plugins/lua/ts_lua_common.h @@ -91,8 +91,8 @@ typedef struct { char script[TS_LUA_MAX_SCRIPT_FNAME_LENGTH]; void *conf_vars[TS_LUA_MAX_CONFIG_VARS_COUNT]; - int _first : 1; // create current instance for 1st ts_lua_main_ctx - int _last : 1; // create current instance for the last ts_lua_main_ctx + unsigned int _first : 1; // create current instance for 1st ts_lua_main_ctx + unsigned int _last : 1; // create current instance for the last ts_lua_main_ctx int remap; int states; @@ -160,10 +160,10 @@ typedef struct { ts_lua_http_ctx *hctx; int64_t to_flush; - int reuse : 1; - int recv_complete : 1; - int send_complete : 1; - int all_ready : 1; + unsigned int reuse : 1; + unsigned int recv_complete : 1; + unsigned int send_complete : 1; + unsigned int all_ready : 1; } ts_lua_http_intercept_ctx; #define TS_LUA_RELEASE_IO_HANDLE(ih) \ diff --git a/plugins/lua/ts_lua_coroutine.h b/plugins/lua/ts_lua_coroutine.h index 831066dc24b..b2fb722fb51 100644 --- a/plugins/lua/ts_lua_coroutine.h +++ b/plugins/lua/ts_lua_coroutine.h @@ -59,7 +59,7 @@ typedef struct async_item { void *data; // private data async_clean cleanup; // cleanup function - int deleted : 1; + unsigned int deleted : 1; } ts_lua_async_item; ts_lua_async_item *ts_lua_async_create_item(TSCont cont, async_clean func, void *d, ts_lua_cont_info *ci); diff --git a/plugins/lua/ts_lua_fetch.h b/plugins/lua/ts_lua_fetch.h index 04c0d47a56c..c4b600cc4d8 100644 --- a/plugins/lua/ts_lua_fetch.h +++ b/plugins/lua/ts_lua_fetch.h @@ -30,8 +30,8 @@ typedef struct { TSIOBufferReader reader; TSFetchSM fch; - int over : 1; - int failed : 1; + unsigned int over : 1; + unsigned int failed : 1; } ts_lua_fetch_info; typedef struct fetch_multi_info { From d263e5f12ed3033d7ce1698b29ddab06fc5e53e2 Mon Sep 17 00:00:00 2001 From: Evan Zelkowitz Date: Wed, 29 Jan 2020 13:56:21 -0700 Subject: [PATCH 2/5] Fixing shadowed variables, both global and local: (#6371) https://lgtm.com/projects/g/apache/trafficserver/?mode=tree&ruleFocus=2157860312 https://lgtm.com/projects/g/apache/trafficserver/?mode=tree&ruleFocus=2156240606 Addresses issue #6352 (cherry picked from commit 2e1202023219457496c2cc49d199f20fc79d5796) --- iocore/hostdb/HostDB.cc | 4 +- iocore/net/SSLUtils.cc | 12 ++-- iocore/net/UnixNetAccept.cc | 6 +- plugins/background_fetch/background_fetch.cc | 4 +- plugins/experimental/memcache/tsmemcache.cc | 8 +-- plugins/header_rewrite/conditions.cc | 16 ++--- plugins/multiplexer/dispatch.cc | 6 +- src/traffic_cache_tool/CacheTool.cc | 10 ++-- src/traffic_logcat/logcat.cc | 12 ++-- src/traffic_server/InkAPI.cc | 10 ++-- src/traffic_server/traffic_server.cc | 18 +++--- src/tscore/IpMap.cc | 62 ++++++++++---------- tools/jtest/jtest.cc | 8 +-- 13 files changed, 88 insertions(+), 88 deletions(-) diff --git a/iocore/hostdb/HostDB.cc b/iocore/hostdb/HostDB.cc index 512781f620a..7c09e3194a3 100644 --- a/iocore/hostdb/HostDB.cc +++ b/iocore/hostdb/HostDB.cc @@ -2209,7 +2209,7 @@ ParseHostLine(Ptr &map, char *l) } void -ParseHostFile(const char *path, unsigned int hostdb_hostfile_check_interval) +ParseHostFile(const char *path, unsigned int hostdb_hostfile_check_interval_parse) { Ptr parsed_hosts_file_ptr; @@ -2229,7 +2229,7 @@ ParseHostFile(const char *path, unsigned int hostdb_hostfile_check_interval) int64_t size = info.st_size + 1; parsed_hosts_file_ptr = new RefCountedHostsFileMap; - parsed_hosts_file_ptr->next_sync_time = ink_time() + hostdb_hostfile_check_interval; + parsed_hosts_file_ptr->next_sync_time = ink_time() + hostdb_hostfile_check_interval_parse; parsed_hosts_file_ptr->HostFileText = static_cast(ats_malloc(size)); if (parsed_hosts_file_ptr->HostFileText) { char *base = parsed_hosts_file_ptr->HostFileText; diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc index 812fd49c656..aaa117d349d 100644 --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -1734,10 +1734,10 @@ SSLWriteBuffer(SSL *ssl, const void *buf, int64_t nbytes, int64_t &nwritten) } int ssl_error = SSL_get_error(ssl, ret); if (ssl_error == SSL_ERROR_SSL && is_debug_tag_set("ssl.error.write")) { - char buf[512]; + char tempbuf[512]; unsigned long e = ERR_peek_last_error(); - ERR_error_string_n(e, buf, sizeof(buf)); - Debug("ssl.error.write", "SSL write returned %d, ssl_error=%d, ERR_get_error=%ld (%s)", ret, ssl_error, e, buf); + ERR_error_string_n(e, tempbuf, sizeof(tempbuf)); + Debug("ssl.error.write", "SSL write returned %d, ssl_error=%d, ERR_get_error=%ld (%s)", ret, ssl_error, e, tempbuf); } return ssl_error; } @@ -1815,10 +1815,10 @@ SSLReadBuffer(SSL *ssl, void *buf, int64_t nbytes, int64_t &nread) } int ssl_error = SSL_get_error(ssl, ret); if (ssl_error == SSL_ERROR_SSL && is_debug_tag_set("ssl.error.read")) { - char buf[512]; + char tempbuf[512]; unsigned long e = ERR_peek_last_error(); - ERR_error_string_n(e, buf, sizeof(buf)); - Debug("ssl.error.read", "SSL read returned %d, ssl_error=%d, ERR_get_error=%ld (%s)", ret, ssl_error, e, buf); + ERR_error_string_n(e, tempbuf, sizeof(tempbuf)); + Debug("ssl.error.read", "SSL read returned %d, ssl_error=%d, ERR_get_error=%ld (%s)", ret, ssl_error, e, tempbuf); } return ssl_error; diff --git a/iocore/net/UnixNetAccept.cc b/iocore/net/UnixNetAccept.cc index 9254d7f0e44..7109a276b35 100644 --- a/iocore/net/UnixNetAccept.cc +++ b/iocore/net/UnixNetAccept.cc @@ -356,11 +356,11 @@ NetAccept::do_blocking_accept(EThread *t) #endif SET_CONTINUATION_HANDLER(vc, (NetVConnHandler)&UnixNetVConnection::acceptEvent); - EThread *t = eventProcessor.assign_thread(opt.etype); - NetHandler *h = get_NetHandler(t); + EThread *localt = eventProcessor.assign_thread(opt.etype); + NetHandler *h = get_NetHandler(localt); // Assign NetHandler->mutex to NetVC vc->mutex = h->mutex; - t->schedule_imm_signal(vc); + localt->schedule_imm_signal(vc); } while (loop); return 1; diff --git a/plugins/background_fetch/background_fetch.cc b/plugins/background_fetch/background_fetch.cc index fbeba7dd318..13aaff4a15e 100644 --- a/plugins/background_fetch/background_fetch.cc +++ b/plugins/background_fetch/background_fetch.cc @@ -531,9 +531,9 @@ cont_handle_response(TSCont contp, TSEvent event, void *edata) TSDebug(PLUGIN_NAME, "Testing: response status code: %d?", status); if (TS_HTTP_STATUS_PARTIAL_CONTENT == status || (config->allow304() && TS_HTTP_STATUS_NOT_MODIFIED == status)) { // Everything looks good so far, add a TXN hook for SEND_RESPONSE_HDR - TSCont contp = TSContCreate(cont_check_cacheable, nullptr); + TSCont localcontp = TSContCreate(cont_check_cacheable, nullptr); - TSHttpTxnHookAdd(txnp, TS_HTTP_SEND_RESPONSE_HDR_HOOK, contp); + TSHttpTxnHookAdd(txnp, TS_HTTP_SEND_RESPONSE_HDR_HOOK, localcontp); } // Release the response MLoc TSHandleMLocRelease(response, TS_NULL_MLOC, resp_hdr); diff --git a/plugins/experimental/memcache/tsmemcache.cc b/plugins/experimental/memcache/tsmemcache.cc index f8e5789227a..42199d0b4e4 100644 --- a/plugins/experimental/memcache/tsmemcache.cc +++ b/plugins/experimental/memcache/tsmemcache.cc @@ -962,13 +962,13 @@ MC::ascii_incr_decr_event(int event, void *data) } header.cas = ink_atomic_increment(&next_cas, 1); { - char *data = nullptr; - int len = 0; + char *localdata = nullptr; + int len = 0; // must be huge, why convert to a counter ?? - if (cwvc->get_single_data((void **)&data, &len) < 0) { + if (cwvc->get_single_data((void **)&localdata, &len) < 0) { goto Lfail; } - uint64_t new_value = xatoull(data, data + len); + uint64_t new_value = xatoull(localdata, localdata + len); if (f.set_incr) { new_value += delta; } else { diff --git a/plugins/header_rewrite/conditions.cc b/plugins/header_rewrite/conditions.cc index 380d270c80e..8bb3848a97f 100644 --- a/plugins/header_rewrite/conditions.cc +++ b/plugins/header_rewrite/conditions.cc @@ -1113,17 +1113,17 @@ ConditionCidr::append_value(std::string &s, const Resources &res) if (addr) { switch (addr->sa_family) { case AF_INET: { - char res[INET_ADDRSTRLEN]; + char resource[INET_ADDRSTRLEN]; struct in_addr ipv4 = reinterpret_cast(addr)->sin_addr; ipv4.s_addr &= _v4_mask.s_addr; - inet_ntop(AF_INET, &ipv4, res, INET_ADDRSTRLEN); - if (res[0]) { - s += res; + inet_ntop(AF_INET, &ipv4, resource, INET_ADDRSTRLEN); + if (resource[0]) { + s += resource; } } break; case AF_INET6: { - char res[INET6_ADDRSTRLEN]; + char resource[INET6_ADDRSTRLEN]; struct in6_addr ipv6 = reinterpret_cast(addr)->sin6_addr; if (_v6_zero_bytes > 0) { @@ -1132,9 +1132,9 @@ ConditionCidr::append_value(std::string &s, const Resources &res) if (_v6_mask != 0xff) { ipv6.s6_addr[16 - _v6_zero_bytes] &= _v6_mask; } - inet_ntop(AF_INET6, &ipv6, res, INET6_ADDRSTRLEN); - if (res[0]) { - s += res; + inet_ntop(AF_INET6, &ipv6, resource, INET6_ADDRSTRLEN); + if (resource[0]) { + s += resource; } } break; } diff --git a/plugins/multiplexer/dispatch.cc b/plugins/multiplexer/dispatch.cc index 675b23a349f..d5f4c3a75fb 100644 --- a/plugins/multiplexer/dispatch.cc +++ b/plugins/multiplexer/dispatch.cc @@ -168,9 +168,9 @@ class Handler if (TSIsDebugTagSet(PLUGIN_TAG) > 0) { const TSIOBuffer buffer = TSIOBufferCreate(); TSHttpHdrPrint(b, l, buffer); - std::string b; - read(buffer, b); - TSDebug(PLUGIN_TAG, "Response header for \"%s\" was:\n%s", url.c_str(), b.c_str()); + std::string buf; + read(buffer, buf); + TSDebug(PLUGIN_TAG, "Response header for \"%s\" was:\n%s", url.c_str(), buf.c_str()); TSIOBufferDestroy(buffer); } } diff --git a/src/traffic_cache_tool/CacheTool.cc b/src/traffic_cache_tool/CacheTool.cc index c5b56d7a3d5..4a68d6dd257 100644 --- a/src/traffic_cache_tool/CacheTool.cc +++ b/src/traffic_cache_tool/CacheTool.cc @@ -520,8 +520,8 @@ Cache::loadSpanConfig(ts::file::path const &path) if (line.empty() || '#' == *line) { continue; } - ts::TextView path = line.take_prefix_if(&isspace); - if (path) { + ts::TextView localpath = line.take_prefix_if(&isspace); + if (localpath) { // After this the line is [size] [id=string] [volume=#] while (line) { ts::TextView value(line.take_prefix_if(&isspace)); @@ -539,7 +539,7 @@ Cache::loadSpanConfig(ts::file::path const &path) } } } - zret = this->loadSpan(ts::file::path(path)); + zret = this->loadSpan(ts::file::path(localpath)); } } } else { @@ -552,7 +552,7 @@ Errata Cache::loadURLs(ts::file::path const &path) { static const ts::TextView TAG_VOL("url"); - ts::URLparser parser; + ts::URLparser loadURLparser; Errata zret; std::error_code ec; @@ -568,7 +568,7 @@ Cache::loadURLs(ts::file::path const &path) std::string url; url.assign(blob.data(), blob.size()); int port_ptr = -1, port_len = -1; - int port = parser.getPort(url, port_ptr, port_len); + int port = loadURLparser.getPort(url, port_ptr, port_len); if (port_ptr >= 0 && port_len > 0) { url.erase(port_ptr, port_len + 1); // get rid of :PORT } diff --git a/src/traffic_logcat/logcat.cc b/src/traffic_logcat/logcat.cc index debbaf0a433..5ac69aa0563 100644 --- a/src/traffic_logcat/logcat.cc +++ b/src/traffic_logcat/logcat.cc @@ -208,14 +208,14 @@ process_file(int in_fd, int out_fd) } static int -open_output_file(char *output_file) +open_output_file(char *output_file_p) { int file_desc = 0; if (!overwrite_existing_file) { - if (access(output_file, F_OK)) { + if (access(output_file_p, F_OK)) { if (errno != ENOENT) { - fprintf(stderr, "Error accessing output file %s: ", output_file); + fprintf(stderr, "Error accessing output file %s: ", output_file_p); perror(nullptr); file_desc = -1; } @@ -223,16 +223,16 @@ open_output_file(char *output_file) fprintf(stderr, "Error, output file %s already exists.\n" "Select a different filename or use the -w flag\n", - output_file); + output_file_p); file_desc = -1; } } if (file_desc == 0) { - file_desc = open(output_file, O_WRONLY | O_TRUNC | O_CREAT, 0640); + file_desc = open(output_file_p, O_WRONLY | O_TRUNC | O_CREAT, 0640); if (file_desc < 0) { - fprintf(stderr, "Error while opening output file %s: ", output_file); + fprintf(stderr, "Error while opening output file %s: ", output_file_p); perror(nullptr); } } diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index eca9973d158..1d93846ae3e 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -6783,19 +6783,19 @@ TSHttpConnectTransparent(sockaddr const *client_addr, sockaddr const *server_add void TSActionCancel(TSAction actionp) { - Action *a; + Action *thisaction; INKContInternal *i; /* This is a hack. Should be handled in ink_types */ if ((uintptr_t)actionp & 0x1) { - a = (Action *)((uintptr_t)actionp - 1); - i = (INKContInternal *)a->continuation; + thisaction = (Action *)((uintptr_t)actionp - 1); + i = (INKContInternal *)thisaction->continuation; i->handle_event_count(EVENT_IMMEDIATE); } else { - a = (Action *)actionp; + thisaction = (Action *)actionp; } - a->cancel(); + thisaction->cancel(); } // Currently no error handling necessary, actionp can be anything. diff --git a/src/traffic_server/traffic_server.cc b/src/traffic_server/traffic_server.cc index b45174ece7b..002459a851b 100644 --- a/src/traffic_server/traffic_server.cc +++ b/src/traffic_server/traffic_server.cc @@ -1641,27 +1641,27 @@ change_uid_gid(const char *user) * This must work without the ability to elevate privilege if the files are accessible without. */ void -bind_outputs(const char *bind_stdout, const char *bind_stderr) +bind_outputs(const char *bind_stdout_p, const char *bind_stderr_p) { int log_fd; unsigned int flags = O_WRONLY | O_APPEND | O_CREAT | O_SYNC; - if (*bind_stdout != 0) { - Debug("log", "binding stdout to %s", bind_stdout); - log_fd = elevating_open(bind_stdout, flags, 0644); + if (*bind_stdout_p != 0) { + Debug("log", "binding stdout to %s", bind_stdout_p); + log_fd = elevating_open(bind_stdout_p, flags, 0644); if (log_fd < 0) { - fprintf(stdout, "[Warning]: TS unable to open log file \"%s\" [%d '%s']\n", bind_stdout, errno, strerror(errno)); + fprintf(stdout, "[Warning]: TS unable to open log file \"%s\" [%d '%s']\n", bind_stdout_p, errno, strerror(errno)); } else { Debug("log", "duping stdout"); dup2(log_fd, STDOUT_FILENO); close(log_fd); } } - if (*bind_stderr != 0) { - Debug("log", "binding stderr to %s", bind_stderr); - log_fd = elevating_open(bind_stderr, O_WRONLY | O_APPEND | O_CREAT | O_SYNC, 0644); + if (*bind_stderr_p != 0) { + Debug("log", "binding stderr to %s", bind_stderr_p); + log_fd = elevating_open(bind_stderr_p, O_WRONLY | O_APPEND | O_CREAT | O_SYNC, 0644); if (log_fd < 0) { - fprintf(stdout, "[Warning]: TS unable to open log file \"%s\" [%d '%s']\n", bind_stderr, errno, strerror(errno)); + fprintf(stdout, "[Warning]: TS unable to open log file \"%s\" [%d '%s']\n", bind_stderr_p, errno, strerror(errno)); } else { Debug("log", "duping stderr"); dup2(log_fd, STDERR_FILENO); diff --git a/src/tscore/IpMap.cc b/src/tscore/IpMap.cc index 3001fc14d78..481ea0307f7 100644 --- a/src/tscore/IpMap.cc +++ b/src/tscore/IpMap.cc @@ -339,22 +339,22 @@ namespace detail N *n = this->lowerBound(rmin); N *x = nullptr; // New node (if any). // Need copies because we will modify these. - Metric min = N::deref(rmin); - Metric max = N::deref(rmax); + Metric localmin = N::deref(rmin); + Metric localmax = N::deref(rmax); // Handle cases involving a node of interest to the left of the // range. if (n) { - if (n->_min < min) { - Metric min_1 = min; + if (n->_min < localmin) { + Metric min_1 = localmin; N::dec(min_1); // dec is OK because min isn't zero. if (n->_max < min_1) { // no overlap or adj. n = next(n); - } else if (n->_max >= max) { // incoming range is covered, just discard. + } else if (n->_max >= localmax) { // incoming range is covered, just discard. return *this; } else if (n->_data != payload) { // different payload, clip range on left. - min = n->_max; - N::inc(min); + localmin = n->_max; + N::inc(localmin); n = next(n); } else { // skew overlap with same payload, use node and continue. x = n; @@ -371,7 +371,7 @@ namespace detail // Careful here -- because max_plus1 might wrap we need to use it only if we can be certain it // didn't. This is done by ordering the range tests so that when max_plus1 is used when we know // there exists a larger value than max. - Metric max_plus1 = max; + Metric max_plus1 = localmax; N::inc(max_plus1); /* Notes: @@ -381,7 +381,7 @@ namespace detail while (n) { if (n->_data == payload) { if (x) { - if (n->_max <= max) { // next range is covered, so we can remove and continue. + if (n->_max <= localmax) { // next range is covered, so we can remove and continue. #if defined(__clang_analyzer__) x->_next = n->_next; // done in @c remove, but CA doesn't realize that. // It's insufficient to assert(x->_next != n) after the remove. @@ -395,52 +395,52 @@ namespace detail return *this; } else { // have the space to finish off the range. - x->setMax(max); + x->setMax(localmax); return *this; } - } else { // not carrying a span. - if (n->_max <= max) { // next range is covered - use it. + } else { // not carrying a span. + if (n->_max <= localmax) { // next range is covered - use it. x = n; - x->setMin(min); + x->setMin(localmin); n = next(n); } else if (n->_min <= max_plus1) { - n->setMin(min); + n->setMin(localmin); return *this; } else { // no overlap, space to complete range. - this->insert_before(n, new N(min, max, payload)); + this->insert_before(n, new N(localmin, localmax, payload)); return *this; } } } else { // different payload if (x) { - if (max < n->_min) { // range ends before n starts, done. - x->setMax(max); + if (localmax < n->_min) { // range ends before n starts, done. + x->setMax(localmax); return *this; - } else if (max <= n->_max) { // range ends before n, done. + } else if (localmax <= n->_max) { // range ends before n, done. x->setMaxMinusOne(n->_min); return *this; } else { // n is contained in range, skip over it. x->setMaxMinusOne(n->_min); - x = nullptr; - min = n->_max; - N::inc(min); // OK because n->_max maximal => next is null. + x = nullptr; + localmin = n->_max; + N::inc(localmin); // OK because n->_max maximal => next is null. n = next(n); } - } else { // no carry node. - if (max < n->_min) { // entirely before next span. - this->insert_before(n, new N(min, max, payload)); + } else { // no carry node. + if (localmax < n->_min) { // entirely before next span. + this->insert_before(n, new N(localmin, localmax, payload)); return *this; } else { - if (min < n->_min) { // leading section, need node. - N *y = new N(min, n->_min, payload); + if (localmin < n->_min) { // leading section, need node. + N *y = new N(localmin, n->_min, payload); y->decrementMax(); this->insert_before(n, y); } - if (max <= n->_max) { // nothing past node + if (localmax <= n->_max) { // nothing past node return *this; } - min = n->_max; - N::inc(min); + localmin = n->_max; + N::inc(localmin); n = next(n); } } @@ -448,9 +448,9 @@ namespace detail } // Invariant: min is larger than any existing range maximum. if (x) { - x->setMax(max); + x->setMax(localmax); } else { - this->append(new N(min, max, payload)); + this->append(new N(localmin, localmax, payload)); } return *this; } diff --git a/tools/jtest/jtest.cc b/tools/jtest/jtest.cc index b8564695109..fa96f4ec6a3 100644 --- a/tools/jtest/jtest.cc +++ b/tools/jtest/jtest.cc @@ -569,11 +569,11 @@ max_limit_fd() } static int -read_ready(int fd) +read_ready(int fd_in) { struct pollfd p; p.events = POLLIN; - p.fd = fd; + p.fd = fd_in; int r = poll(&p, 1, 0); if (r <= 0) { return r; @@ -646,8 +646,8 @@ fast(int sock, int speed, int d) static ink_hrtime elapsed_from_start(int sock) { - ink_hrtime now = ink_get_hrtime_internal(); - return ink_hrtime_diff_msec(now, fd[sock].start); + ink_hrtime timenow = ink_get_hrtime_internal(); + return ink_hrtime_diff_msec(timenow, fd[sock].start); } static int From 31206eb9e2cf304764a68a03370ed7ead819a784 Mon Sep 17 00:00:00 2001 From: Evan Zelkowitz Date: Tue, 28 Jan 2020 13:33:28 -0700 Subject: [PATCH 3/5] Removing always true/false comparisons (#6363) * Removing always true/false comparisons. To me these are the ones that made the most sense to remove. There are some others reported that are: 1) Seem to be false positives due to enum usages for some reason 2) Seemed to me that they should be left in for readability purposes. These ones were constant comparisons but when viewed in the code they were in it made sense to leave them because it fit the code scheme and helped readability Addresses issue #6354 (cherry picked from commit 6d64842e456adc95c0e0b7f123050b8c218d1454) --- iocore/eventsystem/IOBuffer.cc | 2 +- iocore/net/SSLUtils.cc | 2 +- plugins/experimental/memcache/tsmemcache.cc | 2 +- plugins/experimental/uri_signing/uri_signing.c | 9 +++------ proxy/hdrs/HdrTest.cc | 12 ++++-------- proxy/http/HttpSM.cc | 4 ++-- proxy/http/remap/RemapConfig.cc | 5 ----- 7 files changed, 12 insertions(+), 24 deletions(-) diff --git a/iocore/eventsystem/IOBuffer.cc b/iocore/eventsystem/IOBuffer.cc index efa68e3609b..ad95f428f34 100644 --- a/iocore/eventsystem/IOBuffer.cc +++ b/iocore/eventsystem/IOBuffer.cc @@ -137,7 +137,7 @@ MIOBuffer::write(IOBufferBlock const *b, int64_t alen, int64_t offset) continue; } int64_t bytes; - if (len < 0 || len >= max_bytes) { + if (len >= max_bytes) { bytes = max_bytes; } else { bytes = len; diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc index aaa117d349d..71036516c87 100644 --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -409,7 +409,7 @@ ssl_client_hello_callback(SSL *s, int *al, void *arg) * The list in practice only has a single element, so we only consider * the first one. */ - if (remaining != 0 && *p++ == TLSEXT_NAMETYPE_host_name) { + if (*p++ == TLSEXT_NAMETYPE_host_name) { remaining--; /* Now we can finally pull out the byte array with the actual hostname. */ if (remaining > 2) { diff --git a/plugins/experimental/memcache/tsmemcache.cc b/plugins/experimental/memcache/tsmemcache.cc index 42199d0b4e4..1214934050a 100644 --- a/plugins/experimental/memcache/tsmemcache.cc +++ b/plugins/experimental/memcache/tsmemcache.cc @@ -1628,7 +1628,7 @@ TSPluginInit(int argc, const char *argv[]) if (argc < 2) { TSError("[tsmemcache] Usage: tsmemcache.so [accept_port]\n"); goto error; - } else if (argc > 1) { + } else { int port = atoi(argv[1]); if (!port) { TSError("[tsmemcache] bad accept_port '%s'\n", argv[1]); diff --git a/plugins/experimental/uri_signing/uri_signing.c b/plugins/experimental/uri_signing/uri_signing.c index 30c8e4532e3..fe6793bee84 100644 --- a/plugins/experimental/uri_signing/uri_signing.c +++ b/plugins/experimental/uri_signing/uri_signing.c @@ -188,9 +188,7 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) TSHandleMLocRelease(mbuf, TS_NULL_MLOC, ul); PluginDebug("Processing request for %.*s.", url_ct, url); - if (cpi < max_cpi) { - checkpoints[cpi++] = mark_timer(&t); - } + checkpoints[cpi++] = mark_timer(&t); int strip_size = url_ct + 1; strip_uri = (char *)TSmalloc(strip_size); @@ -199,9 +197,8 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) size_t strip_ct; cjose_jws_t *jws = get_jws_from_uri(url, url_ct, package, strip_uri, strip_size, &strip_ct); - if (cpi < max_cpi) { - checkpoints[cpi++] = mark_timer(&t); - } + checkpoints[cpi++] = mark_timer(&t); + int checked_cookies = 0; if (!jws) { check_cookies: diff --git a/proxy/hdrs/HdrTest.cc b/proxy/hdrs/HdrTest.cc index cb767dcedd3..467841698b3 100644 --- a/proxy/hdrs/HdrTest.cc +++ b/proxy/hdrs/HdrTest.cc @@ -600,8 +600,6 @@ HdrTest::test_http_aux(const char *request, const char *response) const char *start; const char *end; - int status = 1; - printf(" <<< MUST BE HAND-VERIFIED FOR FULL BENEFIT >>>\n\n"); /*** (1) parse the request string into req_hdr ***/ @@ -624,7 +622,7 @@ HdrTest::test_http_aux(const char *request, const char *response) if (err == PARSE_RESULT_ERROR) { req_hdr.destroy(); rsp_hdr.destroy(); - return (failures_to_status("test_http_aux", (status == 0))); + return (failures_to_status("test_http_aux", false)); } /*** useless copy to exercise copy function ***/ @@ -665,7 +663,7 @@ HdrTest::test_http_aux(const char *request, const char *response) if (err == PARSE_RESULT_ERROR) { req_hdr.destroy(); rsp_hdr.destroy(); - return (failures_to_status("test_http_aux", (status == 0))); + return (failures_to_status("test_http_aux", false)); } http_parser_clear(&parser); @@ -714,7 +712,7 @@ HdrTest::test_http_aux(const char *request, const char *response) req_hdr.destroy(); rsp_hdr.destroy(); - return (failures_to_status("test_http_aux", (status == 0))); + return (failures_to_status("test_http_aux", false)); } int @@ -1579,8 +1577,6 @@ HdrTest::test_http() int HdrTest::test_http_mutation() { - int status = 1; - bri_box("test_http_mutation"); printf(" <<< MUST BE HAND-VERIFIED FOR FULL BENEFIT>>>\n\n"); @@ -1658,7 +1654,7 @@ HdrTest::test_http_mutation() resp_hdr.destroy(); - return (failures_to_status("test_http_mutation", (status == 0))); + return (failures_to_status("test_http_mutation", false)); } /*------------------------------------------------------------------------- diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index c4e850bc801..d0844a020d6 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -4263,7 +4263,7 @@ HttpSM::parse_range_and_compare(MIMEField *field, int64_t content_length) ; } - if (s < e || start < 0) { + if (s < e) { t_state.range_setup = HttpTransact::RANGE_NONE; goto Lfaild; } @@ -4303,7 +4303,7 @@ HttpSM::parse_range_and_compare(MIMEField *field, int64_t content_length) ; } - if (s < e || end < 0) { + if (s < e) { t_state.range_setup = HttpTransact::RANGE_NONE; goto Lfaild; } diff --git a/proxy/http/remap/RemapConfig.cc b/proxy/http/remap/RemapConfig.cc index 985aee7779c..ecba86328b9 100644 --- a/proxy/http/remap/RemapConfig.cc +++ b/proxy/http/remap/RemapConfig.cc @@ -845,7 +845,6 @@ process_regex_mapping_config(const char *from_host_lower, url_mapping *new_mappi const char *to_host; int to_host_len; int substitution_id; - int substitution_count = 0; int captures; reg_map->to_url_host_template = nullptr; @@ -875,10 +874,6 @@ process_regex_mapping_config(const char *from_host_lower, url_mapping *new_mappi to_host = new_mapping->toURL.host_get(&to_host_len); for (int i = 0; i < (to_host_len - 1); ++i) { if (to_host[i] == '$') { - if (substitution_count > UrlRewrite::MAX_REGEX_SUBS) { - Warning("Cannot have more than %d substitutions in mapping with host [%s]", UrlRewrite::MAX_REGEX_SUBS, from_host_lower); - goto lFail; - } substitution_id = to_host[i + 1] - '0'; if ((substitution_id < 0) || (substitution_id > captures)) { Warning("Substitution id [%c] has no corresponding capture pattern in regex [%s]", to_host[i + 1], from_host_lower); From 4509732ba12bd76fe3c4eb279e6e1f99f674a31b Mon Sep 17 00:00:00 2001 From: Evan Zelkowitz Date: Tue, 28 Jan 2020 12:23:55 -0700 Subject: [PATCH 4/5] Change localtime/gmtime usages to use the threadsafe versions with local storage (#6362) (cherry picked from commit 12c967fe6f82aa070060428567e595e1b83057a2) --- plugins/esi/combo_handler.cc | 7 +++++-- src/traffic_top/traffic_top.cc | 14 ++++++++------ tools/http_load/http_load.c | 4 ++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/plugins/esi/combo_handler.cc b/plugins/esi/combo_handler.cc index 72d27fafba7..0d1edd26566 100644 --- a/plugins/esi/combo_handler.cc +++ b/plugins/esi/combo_handler.cc @@ -1019,7 +1019,8 @@ prepareResponse(InterceptData &int_data, ByteBlockList &body_blocks, string &res resp_header_fields.append("Expires: 0\r\n"); } else { char line_buf[128]; - int line_size = strftime(line_buf, 128, "Expires: %a, %d %b %Y %T GMT\r\n", gmtime(&expires_time)); + struct tm gm_expires_time; + int line_size = strftime(line_buf, 128, "Expires: %a, %d %b %Y %T GMT\r\n", gmtime_r(&expires_time, &gm_expires_time)); resp_header_fields.append(line_buf, line_size); } } @@ -1086,7 +1087,9 @@ writeStandardHeaderFields(InterceptData &int_data, int &n_bytes_written) if (find(HEADER_WHITELIST.begin(), HEADER_WHITELIST.end(), TS_MIME_FIELD_LAST_MODIFIED) == HEADER_WHITELIST.end()) { time_t time_now = static_cast(TShrtime() / 1000000000); // it returns nanoseconds! char last_modified_line[128]; - int last_modified_line_size = strftime(last_modified_line, 128, "Last-Modified: %a, %d %b %Y %T GMT\r\n", gmtime(&time_now)); + struct tm gmnow; + int last_modified_line_size = + strftime(last_modified_line, 128, "Last-Modified: %a, %d %b %Y %T GMT\r\n", gmtime_r(&time_now, &gmnow)); if (TSIOBufferWrite(int_data.output.buffer, last_modified_line, last_modified_line_size) == TS_ERROR) { LOG_ERROR("Error while writing last-modified fields"); return false; diff --git a/src/traffic_top/traffic_top.cc b/src/traffic_top/traffic_top.cc index 625589c5eb8..d33d682e1c0 100644 --- a/src/traffic_top/traffic_top.cc +++ b/src/traffic_top/traffic_top.cc @@ -228,10 +228,11 @@ help(const string &host, const string &version) while (true) { clear(); - time_t now = time(nullptr); - struct tm *nowtm = localtime(&now); + time_t now = time(nullptr); + struct tm nowtm; char timeBuf[32]; - strftime(timeBuf, sizeof(timeBuf), "%H:%M:%S", nowtm); + localtime_r(&now, &nowtm); + strftime(timeBuf, sizeof(timeBuf), "%H:%M:%S", &nowtm); // clear(); attron(A_BOLD); @@ -467,10 +468,11 @@ main(int argc, const char **argv) attron(A_BOLD); string version; - time_t now = time(nullptr); - struct tm *nowtm = localtime(&now); + time_t now = time(nullptr); + struct tm nowtm; char timeBuf[32]; - strftime(timeBuf, sizeof(timeBuf), "%H:%M:%S", nowtm); + localtime_r(&now, &nowtm); + strftime(timeBuf, sizeof(timeBuf), "%H:%M:%S", &nowtm); stats.getStat("version", version); mvprintw(23, 0, "%-20.20s %30s (q)uit (h)elp (%c)bsolute ", host.c_str(), page_alt.c_str(), absolute ? 'A' : 'a'); diff --git a/tools/http_load/http_load.c b/tools/http_load/http_load.c index 59bd5dd6bae..8d89962b592 100644 --- a/tools/http_load/http_load.c +++ b/tools/http_load/http_load.c @@ -2760,9 +2760,9 @@ idle_connection(ClientData client_data, struct timeval *nowP __attribute__((unus int cnum; struct timeval tv; char strTime[32]; - + struct tm localtv; gettimeofday(&tv, NULL); - strftime(strTime, 32, "%T", localtime(&tv.tv_sec)); + strftime(strTime, 32, "%T", localtime_r(&tv.tv_sec, &localtv)); cnum = client_data.i; connections[cnum].idle_timer = (Timer *)0; From 8c543f69dda49fec04ed7dd5804be7c5a45ed90d Mon Sep 17 00:00:00 2001 From: Evan Zelkowitz Date: Mon, 27 Jan 2020 18:33:08 -0700 Subject: [PATCH 5/5] Add header guard (#6358) Header guard for EnumDescriptor.h (cherry picked from commit 0e64d36ecfea0a93ccdf9b2d2d1cc000c7273852) --- include/tscore/EnumDescriptor.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/tscore/EnumDescriptor.h b/include/tscore/EnumDescriptor.h index 485a5c3db59..f1c4b5075a4 100644 --- a/include/tscore/EnumDescriptor.h +++ b/include/tscore/EnumDescriptor.h @@ -19,6 +19,8 @@ limitations under the License. */ +#pragma once + #include #include