From 3925d2f7eaa7a333e0fa8598487aebdc3a299b58 Mon Sep 17 00:00:00 2001 From: Randall Meyer Date: Mon, 21 Mar 2022 11:07:14 -0700 Subject: [PATCH] Remove null check before ats_free calls ats_free already does null checks --- iocore/cache/CacheHosting.cc | 12 +++--------- iocore/cache/P_CacheInternal.h | 6 +++--- iocore/net/ALPNSupport.cc | 9 ++++----- iocore/net/OCSPStapling.cc | 19 ++++++------------- iocore/net/SSLNextProtocolSet.cc | 6 ++---- iocore/net/TLSTunnelSupport.cc | 5 ++--- iocore/net/UnixUDPNet.cc | 2 +- lib/records/RecCore.cc | 6 +++--- lib/records/RecUtils.cc | 4 +--- lib/records/test_RecProcess.i | 2 +- plugins/experimental/memcache/tsmemcache.cc | 12 ++++++------ proxy/http/HttpSM.cc | 8 +++----- proxy/http/remap/RemapConfig.cc | 9 ++++----- proxy/http/remap/UrlMapping.h | 6 ++---- proxy/http/remap/UrlRewrite.cc | 4 +--- .../remap/unit-tests/nexthop_test_stubs.cc | 6 +++--- proxy/http2/Http2Stream.cc | 5 ++--- proxy/http3/QPACK.cc | 6 ++---- proxy/logging/LogFormat.cc | 18 ++++++++---------- src/traffic_server/InkAPI.cc | 18 +++++++----------- 20 files changed, 64 insertions(+), 99 deletions(-) diff --git a/iocore/cache/CacheHosting.cc b/iocore/cache/CacheHosting.cc index 41c995742ff..e98793fa18e 100644 --- a/iocore/cache/CacheHosting.cc +++ b/iocore/cache/CacheHosting.cc @@ -490,18 +490,14 @@ CacheHostRecord::Init(matcher_line *line_info, CacheType typ) const char *errptr = "A volume number expected"; RecSignalWarning(REC_SIGNAL_CONFIG_ERROR, "%s discarding %s entry at line %d :%s", "[CacheHosting]", config_file, line_info->line_num, errptr); - if (val != nullptr) { - ats_free(val); - } + ats_free(val); return -1; } } if ((*s < '0') || (*s > '9')) { RecSignalWarning(REC_SIGNAL_CONFIG_ERROR, "%s discarding %s entry at line %d : bad token [%c]", "[CacheHosting]", config_file, line_info->line_num, *s); - if (val != nullptr) { - ats_free(val); - } + ats_free(val); return -1; } s++; @@ -537,9 +533,7 @@ CacheHostRecord::Init(matcher_line *line_info, CacheType typ) if (!is_vol_present) { RecSignalWarning(REC_SIGNAL_CONFIG_ERROR, "%s discarding %s entry at line %d : bad volume number [%d]", "[CacheHosting]", config_file, line_info->line_num, volume_number); - if (val != nullptr) { - ats_free(val); - } + ats_free(val); return -1; } if (c == '\0') { diff --git a/iocore/cache/P_CacheInternal.h b/iocore/cache/P_CacheInternal.h index 756eb14deef..8630d3a064f 100644 --- a/iocore/cache/P_CacheInternal.h +++ b/iocore/cache/P_CacheInternal.h @@ -614,9 +614,9 @@ free_CacheVC(CacheVC *cont) cont->blocks.clear(); cont->writer_buf.clear(); cont->alternate_index = CACHE_ALT_INDEX_DEFAULT; - if (cont->scan_vol_map) { - ats_free(cont->scan_vol_map); - } + + ats_free(cont->scan_vol_map); + memset((char *)&cont->vio, 0, cont->size_to_init); #ifdef CACHE_STAT_PAGES ink_assert(!cont->stat_link.next && !cont->stat_link.prev); diff --git a/iocore/net/ALPNSupport.cc b/iocore/net/ALPNSupport.cc index 69b8b1c24d7..1e9bc8aacbe 100644 --- a/iocore/net/ALPNSupport.cc +++ b/iocore/net/ALPNSupport.cc @@ -57,11 +57,10 @@ ALPNSupport::unbind(SSL *ssl) void ALPNSupport::clear() { - if (npn) { - ats_free(npn); - npn = nullptr; - npnsz = 0; - } + ats_free(npn); + npn = nullptr; + npnsz = 0; + npnSet = nullptr; npnEndpoint = nullptr; } diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index d5721650239..da5a8c16356 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -77,12 +77,10 @@ certinfo_map_free(void * /*parent*/, void *ptr, CRYPTO_EX_DATA * /*ad*/, int /*i if (cinf->uri) { OPENSSL_free(cinf->uri); } - if (cinf->certname) { - ats_free(cinf->certname); - } - if (cinf->user_agent) { - ats_free(cinf->user_agent); - } + + ats_free(cinf->certname); + ats_free(cinf->user_agent); + ink_mutex_destroy(&cinf->stapling_mutex); OPENSSL_free(cinf); } @@ -295,13 +293,8 @@ ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, const char *certname, const cha OCSP_CERTID_free(cinf->cid); } - if (cinf->certname) { - ats_free(cinf->certname); - } - - if (cinf->user_agent) { - ats_free(cinf->user_agent); - } + ats_free(cinf->certname); + ats_free(cinf->user_agent); if (cinf) { OPENSSL_free(cinf); diff --git a/iocore/net/SSLNextProtocolSet.cc b/iocore/net/SSLNextProtocolSet.cc index 99df1f8a4db..903144e09d5 100644 --- a/iocore/net/SSLNextProtocolSet.cc +++ b/iocore/net/SSLNextProtocolSet.cc @@ -49,10 +49,8 @@ SSLNextProtocolSet::create_npn_advertisement(const SessionProtocolSet &enabled, const SSLNextProtocolSet::NextProtocolEndpoint *ep; unsigned char *advertised; - if (*npn) { - ats_free(*npn); - *npn = nullptr; - } + ats_free(*npn); + *npn = nullptr; *len = 0; for (ep = endpoints.head; ep != nullptr; ep = endpoints.next(ep)) { diff --git a/iocore/net/TLSTunnelSupport.cc b/iocore/net/TLSTunnelSupport.cc index 77dbdd8dff8..b050c8c4417 100644 --- a/iocore/net/TLSTunnelSupport.cc +++ b/iocore/net/TLSTunnelSupport.cc @@ -67,10 +67,9 @@ TLSTunnelSupport::set_tunnel_destination(const std::string_view &destination, SN _tunnel_type = type; _tunnel_prewarm = prewarm; + ats_free(_tunnel_host); + auto pos = destination.find(":"); - if (nullptr != _tunnel_host) { - ats_free(_tunnel_host); - } if (pos != std::string::npos) { _tunnel_port = std::stoi(destination.substr(pos + 1).data()); _tunnel_host = ats_strndup(destination.substr(0, pos).data(), pos); diff --git a/iocore/net/UnixUDPNet.cc b/iocore/net/UnixUDPNet.cc index c2f5177d10c..b3141728949 100644 --- a/iocore/net/UnixUDPNet.cc +++ b/iocore/net/UnixUDPNet.cc @@ -505,7 +505,7 @@ UDPReadContinuation::readPollEvent(int event_, Event *e) * *actual_len = recvfrom(fd,addr,buf->end(),len) * if successful then * buf->fill(*actual_len); - * return ACTION_RESULT_DONE + * return ACTION_RESULT_DONE * else if nothing read * *actual_len is 0 * create "UDP read continuation" C with 'cont's lock diff --git a/lib/records/RecCore.cc b/lib/records/RecCore.cc index fcb6474cbce..4fa791d1e6d 100644 --- a/lib/records/RecCore.cc +++ b/lib/records/RecCore.cc @@ -898,9 +898,9 @@ RecRegisterConfig(RecT rec_type, const char *name, RecDataT data_type, RecData d // Note: do not modify 'record->config_meta.update_required' r->config_meta.update_type = update_type; r->config_meta.check_type = check_type; - if (r->config_meta.check_expr) { - ats_free(r->config_meta.check_expr); - } + + ats_free(r->config_meta.check_expr); + r->config_meta.check_expr = ats_strdup(check_expr); r->config_meta.update_cb_list = nullptr; r->config_meta.access_type = access_type; diff --git a/lib/records/RecUtils.cc b/lib/records/RecUtils.cc index e0cdbb80a7e..94280413bf2 100644 --- a/lib/records/RecUtils.cc +++ b/lib/records/RecUtils.cc @@ -129,9 +129,7 @@ RecDataSet(RecDataT data_type, RecData *data_dst, RecData *data_src) } } else if (((data_dst->rec_string) && (strcmp(data_dst->rec_string, data_src->rec_string) != 0)) || ((data_dst->rec_string == nullptr) && (data_src->rec_string != nullptr))) { - if (data_dst->rec_string) { - ats_free(data_dst->rec_string); - } + ats_free(data_dst->rec_string); data_dst->rec_string = ats_strdup(data_src->rec_string); rec_set = true; diff --git a/lib/records/test_RecProcess.i b/lib/records/test_RecProcess.i index 963c45dc91f..e9c3c918ed9 100644 --- a/lib/records/test_RecProcess.i +++ b/lib/records/test_RecProcess.i @@ -46,7 +46,7 @@ void RecDumpRecordsHt(RecT rec_type); do { \ RecString rec_string = 0; \ if (RecGetRecordString_Xmalloc("proxy.config.parse_"name, &rec_string) != REC_ERR_FAIL) { \ - if (rec_string) ats_free(rec_string); \ + ats_free(rec_string); \ printf(" parse_"name": FAIL\n"); \ failures++; \ } else { \ diff --git a/plugins/experimental/memcache/tsmemcache.cc b/plugins/experimental/memcache/tsmemcache.cc index 6943b57017e..9bc11981df3 100644 --- a/plugins/experimental/memcache/tsmemcache.cc +++ b/plugins/experimental/memcache/tsmemcache.cc @@ -179,9 +179,9 @@ MC::die() if (cbuf) { free_MIOBuffer(cbuf); } - if (tbuf) { - ats_free(tbuf); - } + + ats_free(tbuf); + mutex = NULL; theMCAllocator.free(this); return EVENT_DONE; @@ -351,9 +351,9 @@ MC::read_from_client() cbuf->clear(); } ink_assert(!crvc && !cwvc); - if (tbuf) { - ats_free(tbuf); - } + + ats_free(tbuf); + return TS_SET_CALL(&MC::read_from_client_event, VC_EVENT_READ_READY, rvio); } diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index fbecbe70a6b..3e83c74da15 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -7249,11 +7249,9 @@ HttpSM::kill_this() HTTP_SM_SET_DEFAULT_HANDLER(nullptr); - if (redirect_url != nullptr) { - ats_free((void *)redirect_url); - redirect_url = nullptr; - redirect_url_len = 0; - } + ats_free(redirect_url); + redirect_url = nullptr; + redirect_url_len = 0; #ifdef USE_HTTP_DEBUG_LISTS ink_mutex_acquire(&debug_sm_list_mutex); diff --git a/proxy/http/remap/RemapConfig.cc b/proxy/http/remap/RemapConfig.cc index 2089fb4bd79..61afbd70221 100644 --- a/proxy/http/remap/RemapConfig.cc +++ b/proxy/http/remap/RemapConfig.cc @@ -899,11 +899,10 @@ process_regex_mapping_config(const char *from_host_lower, url_mapping *new_mappi return true; lFail: - if (reg_map->to_url_host_template) { - ats_free(reg_map->to_url_host_template); - reg_map->to_url_host_template = nullptr; - reg_map->to_url_host_template_len = 0; - } + ats_free(reg_map->to_url_host_template); + reg_map->to_url_host_template = nullptr; + reg_map->to_url_host_template_len = 0; + return false; } diff --git a/proxy/http/remap/UrlMapping.h b/proxy/http/remap/UrlMapping.h index c826c4acebe..bb25e36ad75 100644 --- a/proxy/http/remap/UrlMapping.h +++ b/proxy/http/remap/UrlMapping.h @@ -64,10 +64,8 @@ class redirect_tag_str ~redirect_tag_str() { type = 0; - if (chunk_str) { - ats_free(chunk_str); - chunk_str = nullptr; - } + ats_free(chunk_str); + chunk_str = nullptr; } redirect_tag_str *next = nullptr; diff --git a/proxy/http/remap/UrlRewrite.cc b/proxy/http/remap/UrlRewrite.cc index 85dd19f5fdc..96de22b3a14 100644 --- a/proxy/http/remap/UrlRewrite.cc +++ b/proxy/http/remap/UrlRewrite.cc @@ -957,9 +957,7 @@ UrlRewrite::_destroyList(RegexMappingList &mappings) RegexMapping *list_iter; while ((list_iter = mappings.pop()) != nullptr) { delete list_iter->url_map; - if (list_iter->to_url_host_template) { - ats_free(list_iter->to_url_host_template); - } + ats_free(list_iter->to_url_host_template); delete list_iter; } mappings.clear(); diff --git a/proxy/http/remap/unit-tests/nexthop_test_stubs.cc b/proxy/http/remap/unit-tests/nexthop_test_stubs.cc index 5213eb9367f..b72183b21a8 100644 --- a/proxy/http/remap/unit-tests/nexthop_test_stubs.cc +++ b/proxy/http/remap/unit-tests/nexthop_test_stubs.cc @@ -102,9 +102,9 @@ build_request(int64_t sm_id, HttpSM *sm, sockaddr_in *ip, const char *os_hostnam } sm->t_state.request_data.hdr = new HTTPHdr(); sm->t_state.request_data.hdr->create(HTTP_TYPE_REQUEST, myHeap); - if (sm->t_state.request_data.hostname_str != nullptr) { - ats_free(sm->t_state.request_data.hostname_str); - } + + ats_free(sm->t_state.request_data.hostname_str); + sm->t_state.request_data.hostname_str = ats_strdup(os_hostname); sm->t_state.request_data.xact_start = time(nullptr); ink_zero(sm->t_state.request_data.src_ip); diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc index a453c237790..a2bb06f3230 100644 --- a/proxy/http2/Http2Stream.cc +++ b/proxy/http2/Http2Stream.cc @@ -130,9 +130,8 @@ Http2Stream::~Http2Stream() read_vio.mutex.clear(); write_vio.mutex.clear(); - if (header_blocks) { - ats_free(header_blocks); - } + ats_free(header_blocks); + _clear_timers(); clear_io_events(); http_parser_clear(&http_parser); diff --git a/proxy/http3/QPACK.cc b/proxy/http3/QPACK.cc index ca937f751d7..6aec8f86614 100644 --- a/proxy/http3/QPACK.cc +++ b/proxy/http3/QPACK.cc @@ -1823,10 +1823,8 @@ QPACK::DynamicTableStorage::DynamicTableStorage(uint16_t size) : _head(size * 2 QPACK::DynamicTableStorage::~DynamicTableStorage() { - if (this->_data) { - ats_free(this->_data); - this->_data = nullptr; - } + ats_free(this->_data); + this->_data = nullptr; } void diff --git a/proxy/logging/LogFormat.cc b/proxy/logging/LogFormat.cc index 330bc190576..c1f1901892d 100644 --- a/proxy/logging/LogFormat.cc +++ b/proxy/logging/LogFormat.cc @@ -142,21 +142,19 @@ LogFormat::init_variables(const char *name, const char *fieldlist_str, const cha m_agg_marshal_space = static_cast(ats_malloc(m_field_count * INK_MIN_ALIGN)); } - if (m_name_str) { - ats_free(m_name_str); - m_name_str = nullptr; - m_name_id = 0; - } + ats_free(m_name_str); + m_name_str = nullptr; + m_name_id = 0; + if (name) { m_name_str = ats_strdup(name); m_name_id = id_from_name(m_name_str); } - if (m_fieldlist_str) { - ats_free(m_fieldlist_str); - m_fieldlist_str = nullptr; - m_fieldlist_id = 0; - } + ats_free(m_fieldlist_str); + m_fieldlist_str = nullptr; + m_fieldlist_id = 0; + if (fieldlist_str) { m_fieldlist_str = ats_strdup(fieldlist_str); m_fieldlist_id = id_from_name(m_fieldlist_str); diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index f00878ce3fd..f8d92e722ee 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -850,12 +850,10 @@ FileImpl::fclose() m_mode = CLOSED; } - if (m_buf) { - ats_free(m_buf); - m_buf = nullptr; - m_bufsize = 0; - m_bufpos = 0; - } + ats_free(m_buf); + m_buf = nullptr; + m_bufsize = 0; + m_bufpos = 0; } ssize_t @@ -8190,11 +8188,9 @@ TSHttpTxnRedirectUrlSet(TSHttpTxn txnp, const char *url, const int url_len) HttpSM *sm = (HttpSM *)txnp; - if (sm->redirect_url != nullptr) { - ats_free(sm->redirect_url); - sm->redirect_url = nullptr; - sm->redirect_url_len = 0; - } + ats_free(sm->redirect_url); + sm->redirect_url = nullptr; + sm->redirect_url_len = 0; sm->redirect_url = (char *)url; sm->redirect_url_len = url_len;