From 30be5cbdb025bca317f5aa2d12b1c8f082e8af89 Mon Sep 17 00:00:00 2001 From: Uttam Pawar Date: Fri, 12 Oct 2018 13:52:52 -0700 Subject: [PATCH] src: memory management using smart pointer Introduced use of smart pointers instead of MallocedBuffer to manage memory allocated in the cares library. PR-URL: https://github.com/nodejs/node/pull/23628 Reviewed-By: Anna Henningsen Reviewed-By: Joyee Cheung Reviewed-By: Colin Ihrig Reviewed-By: Refael Ackermann --- src/cares_wrap.cc | 109 +++++++++++++++++++++++----------------------- 1 file changed, 54 insertions(+), 55 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 1a83a1cf7adf09..d9f6485edfb507 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1061,118 +1061,117 @@ int ParseSoaReply(Environment* env, EscapableHandleScope handle_scope(env->isolate()); auto context = env->context(); - /* Can't use ares_parse_soa_reply() here which can only parse single record */ - unsigned int ancount = cares_get_16bit(buf + 6); + // Manage memory using standardard smart pointer std::unique_tr + struct AresDeleter { + void operator()(char* ptr) const noexcept { ares_free_string(ptr); } + }; + using ares_unique_ptr = std::unique_ptr; + + // Can't use ares_parse_soa_reply() here which can only parse single record + const unsigned int ancount = cares_get_16bit(buf + 6); unsigned char* ptr = buf + NS_HFIXEDSZ; - char* name; - char* rr_name; + char* name_temp; long temp_len; // NOLINT(runtime/int) - int status = ares_expand_name(ptr, buf, len, &name, &temp_len); + int status = ares_expand_name(ptr, buf, len, &name_temp, &temp_len); + const ares_unique_ptr name(name_temp); if (status != ARES_SUCCESS) { - /* returns EBADRESP in case of invalid input */ + // returns EBADRESP in case of invalid input return status == ARES_EBADNAME ? ARES_EBADRESP : status; } if (ptr + temp_len + NS_QFIXEDSZ > buf + len) { - free(name); return ARES_EBADRESP; } ptr += temp_len + NS_QFIXEDSZ; for (unsigned int i = 0; i < ancount; i++) { - status = ares_expand_name(ptr, buf, len, &rr_name, &temp_len); + char* rr_name_temp; + long rr_temp_len; // NOLINT(runtime/int) + int status2 = ares_expand_name(ptr, buf, len, &rr_name_temp, &rr_temp_len); + const ares_unique_ptr rr_name(rr_name_temp); - if (status != ARES_SUCCESS) - break; + if (status2 != ARES_SUCCESS) + return status2 == ARES_EBADNAME ? ARES_EBADRESP : status2; - ptr += temp_len; + ptr += rr_temp_len; if (ptr + NS_RRFIXEDSZ > buf + len) { - free(rr_name); - status = ARES_EBADRESP; - break; + return ARES_EBADRESP; } const int rr_type = cares_get_16bit(ptr); const int rr_len = cares_get_16bit(ptr + 8); ptr += NS_RRFIXEDSZ; - /* only need SOA */ + // only need SOA if (rr_type == ns_t_soa) { - ares_soa_reply soa; - - status = ares_expand_name(ptr, buf, len, &soa.nsname, &temp_len); - if (status != ARES_SUCCESS) { - free(rr_name); - break; + char* nsname_temp; + long nsname_temp_len; // NOLINT(runtime/int) + + int status3 = ares_expand_name(ptr, buf, len, + &nsname_temp, + &nsname_temp_len); + const ares_unique_ptr nsname(nsname_temp); + if (status3 != ARES_SUCCESS) { + return status3 == ARES_EBADNAME ? ARES_EBADRESP : status3; } - ptr += temp_len; - - status = ares_expand_name(ptr, buf, len, &soa.hostmaster, &temp_len); - if (status != ARES_SUCCESS) { - free(rr_name); - free(soa.nsname); - break; + ptr += nsname_temp_len; + + char* hostmaster_temp; + long hostmaster_temp_len; // NOLINT(runtime/int) + int status4 = ares_expand_name(ptr, buf, len, + &hostmaster_temp, + &hostmaster_temp_len); + const ares_unique_ptr hostmaster(hostmaster_temp); + if (status4 != ARES_SUCCESS) { + return status4 == ARES_EBADNAME ? ARES_EBADRESP : status4; } - ptr += temp_len; + ptr += hostmaster_temp_len; if (ptr + 5 * 4 > buf + len) { - free(rr_name); - free(soa.nsname); - free(soa.hostmaster); - status = ARES_EBADRESP; - break; + return ARES_EBADRESP; } - soa.serial = cares_get_32bit(ptr + 0 * 4); - soa.refresh = cares_get_32bit(ptr + 1 * 4); - soa.retry = cares_get_32bit(ptr + 2 * 4); - soa.expire = cares_get_32bit(ptr + 3 * 4); - soa.minttl = cares_get_32bit(ptr + 4 * 4); + const unsigned int serial = cares_get_32bit(ptr + 0 * 4); + const unsigned int refresh = cares_get_32bit(ptr + 1 * 4); + const unsigned int retry = cares_get_32bit(ptr + 2 * 4); + const unsigned int expire = cares_get_32bit(ptr + 3 * 4); + const unsigned int minttl = cares_get_32bit(ptr + 4 * 4); Local soa_record = Object::New(env->isolate()); soa_record->Set(context, env->nsname_string(), - OneByteString(env->isolate(), soa.nsname)).FromJust(); + OneByteString(env->isolate(), nsname.get())).FromJust(); soa_record->Set(context, env->hostmaster_string(), OneByteString(env->isolate(), - soa.hostmaster)).FromJust(); + hostmaster.get())).FromJust(); soa_record->Set(context, env->serial_string(), - Integer::New(env->isolate(), soa.serial)).FromJust(); + Integer::New(env->isolate(), serial)).FromJust(); soa_record->Set(context, env->refresh_string(), - Integer::New(env->isolate(), soa.refresh)).FromJust(); + Integer::New(env->isolate(), refresh)).FromJust(); soa_record->Set(context, env->retry_string(), - Integer::New(env->isolate(), soa.retry)).FromJust(); + Integer::New(env->isolate(), retry)).FromJust(); soa_record->Set(context, env->expire_string(), - Integer::New(env->isolate(), soa.expire)).FromJust(); + Integer::New(env->isolate(), expire)).FromJust(); soa_record->Set(context, env->minttl_string(), - Integer::New(env->isolate(), soa.minttl)).FromJust(); + Integer::New(env->isolate(), minttl)).FromJust(); soa_record->Set(context, env->type_string(), env->dns_soa_string()).FromJust(); - free(soa.nsname); - free(soa.hostmaster); *ret = handle_scope.Escape(soa_record); break; } - free(rr_name); ptr += rr_len; } - free(name); - - if (status != ARES_SUCCESS) { - return status == ARES_EBADNAME ? ARES_EBADRESP : status; - } - return ARES_SUCCESS; }