Skip to content

Commit

Permalink
free: instead of Curl_safefree()
Browse files Browse the repository at this point in the history
Since we just started make use of free(NULL) in order to simplify code,
this change takes it a step further and:

- converts lots of Curl_safefree() calls to good old free()
- makes Curl_safefree() not check the pointer before free()

The (new) rule of thumb is: if you really want a function call that
frees a pointer and then assigns it to NULL, then use Curl_safefree().
But we will prefer just using free() from now on.
  • Loading branch information
bagder committed Mar 16, 2015
1 parent 9e66160 commit 0f4a03c
Show file tree
Hide file tree
Showing 35 changed files with 202 additions and 202 deletions.
4 changes: 2 additions & 2 deletions lib/asyn-ares.c
Expand Up @@ -533,15 +533,15 @@ Curl_addrinfo *Curl_resolver_getaddrinfo(struct connectdata *conn,
bufp = strdup(hostname);
if(bufp) {
struct ResolverResults *res = NULL;
Curl_safefree(conn->async.hostname);
free(conn->async.hostname);
conn->async.hostname = bufp;
conn->async.port = port;
conn->async.done = FALSE; /* not done */
conn->async.status = 0; /* clear */
conn->async.dns = NULL; /* clear */
res = calloc(sizeof(struct ResolverResults),1);
if(!res) {
Curl_safefree(conn->async.hostname);
free(conn->async.hostname);
conn->async.hostname = NULL;
return NULL;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/asyn-thread.c
Expand Up @@ -393,7 +393,7 @@ static bool init_resolve_thread (struct connectdata *conn,
if(!init_thread_sync_data(td, hostname, port, hints))
goto err_exit;

Curl_safefree(conn->async.hostname);
free(conn->async.hostname);
conn->async.hostname = strdup(hostname);
if(!conn->async.hostname)
goto err_exit;
Expand Down
2 changes: 1 addition & 1 deletion lib/base64.c
Expand Up @@ -149,7 +149,7 @@ CURLcode Curl_base64_decode(const char *src,
for(i = 0; i < numQuantums; i++) {
size_t result = decodeQuantum(pos, src);
if(!result) {
Curl_safefree(newstr);
free(newstr);

return CURLE_BAD_CONTENT_ENCODING;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/bundles.c
Expand Up @@ -6,7 +6,7 @@
* \___|\___/|_| \_\_____|
*
* Copyright (C) 2012, Linus Nielsen Feltzing, <linus@haxx.se>
* Copyright (C) 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
* Copyright (C) 2012-2015, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
Expand Down Expand Up @@ -74,7 +74,7 @@ void Curl_bundle_destroy(struct connectbundle *cb_ptr)
Curl_llist_destroy(cb_ptr->conn_list, NULL);
cb_ptr->conn_list = NULL;
}
Curl_safefree(cb_ptr);
free(cb_ptr);
}

/* Add a connection to a bundle */
Expand Down
2 changes: 1 addition & 1 deletion lib/cookie.c
Expand Up @@ -949,7 +949,7 @@ struct CookieInfo *Curl_cookie_init(struct SessionHandle *data,
return c;

fail:
Curl_safefree(line);
free(line);
if(!inc)
/* Only clean up if we allocated it here, as the original could still be in
* use by a share handle */
Expand Down
6 changes: 4 additions & 2 deletions lib/curl_multibyte.c
Expand Up @@ -47,7 +47,8 @@ wchar_t *Curl_convert_UTF8_to_wchar(const char *str_utf8)
if(str_w) {
if(MultiByteToWideChar(CP_UTF8, 0, str_utf8, -1, str_w,
str_w_len) == 0) {
Curl_safefree(str_w);
free(str_w);
return NULL;
}
}
}
Expand All @@ -68,7 +69,8 @@ char *Curl_convert_wchar_to_UTF8(const wchar_t *str_w)
if(str_utf8) {
if(WideCharToMultiByte(CP_UTF8, 0, str_w, -1, str_utf8, str_utf8_len,
NULL, FALSE) == 0) {
Curl_safefree(str_utf8);
free(str_utf8);
return NULL;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/curl_ntlm.c
Expand Up @@ -173,7 +173,7 @@ CURLcode Curl_output_ntlm(struct connectdata *conn, bool proxy)
return result;

if(base64) {
Curl_safefree(*allocuserpwd);
free(*allocuserpwd);
*allocuserpwd = aprintf("%sAuthorization: NTLM %s\r\n",
proxy ? "Proxy-" : "",
base64);
Expand All @@ -193,7 +193,7 @@ CURLcode Curl_output_ntlm(struct connectdata *conn, bool proxy)
return result;

if(base64) {
Curl_safefree(*allocuserpwd);
free(*allocuserpwd);
*allocuserpwd = aprintf("%sAuthorization: NTLM %s\r\n",
proxy ? "Proxy-" : "",
base64);
Expand Down
5 changes: 2 additions & 3 deletions lib/curl_ntlm_core.c
Expand Up @@ -618,7 +618,7 @@ CURLcode Curl_ntlm_core_mk_ntlmv2_hash(const char *user, size_t userlen,
result = Curl_hmac_md5(ntlmhash, 16, identity, curlx_uztoui(identity_len),
ntlmv2hash);

Curl_safefree(identity);
free(identity);

return result;
}
Expand Down Expand Up @@ -705,8 +705,7 @@ CURLcode Curl_ntlm_core_mk_ntlmv2_resp(unsigned char *ntlmv2hash,
result = Curl_hmac_md5(ntlmv2hash, NTLM_HMAC_MD5_LEN, ptr + 8,
NTLMv2_BLOB_LEN + 8, hmac_output);
if(result) {
Curl_safefree(ptr);

free(ptr);
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/curl_ntlm_msgs.c
Expand Up @@ -758,7 +758,7 @@ CURLcode Curl_sasl_create_ntlm_type3_message(struct SessionHandle *data,
ntlm_print_hex(stderr, (char *)&ntlmbuf[ntrespoff], ntresplen);
});

Curl_safefree(ntlmv2resp);/* Free the dynamic buffer allocated for NTLMv2 */
free(ntlmv2resp);/* Free the dynamic buffer allocated for NTLMv2 */

#endif

Expand Down
18 changes: 9 additions & 9 deletions lib/curl_ntlm_wb.c
Expand Up @@ -105,9 +105,9 @@ void Curl_ntlm_wb_cleanup(struct connectdata *conn)
conn->ntlm_auth_hlpr_pid = 0;
}

Curl_safefree(conn->challenge_header);
free(conn->challenge_header);
conn->challenge_header = NULL;
Curl_safefree(conn->response_header);
free(conn->response_header);
conn->response_header = NULL;
}

Expand Down Expand Up @@ -244,13 +244,13 @@ static CURLcode ntlm_wb_init(struct connectdata *conn, const char *userp)
sclose(sockfds[1]);
conn->ntlm_auth_hlpr_socket = sockfds[0];
conn->ntlm_auth_hlpr_pid = child_pid;
Curl_safefree(domain);
Curl_safefree(ntlm_auth_alloc);
free(domain);
free(ntlm_auth_alloc);
return CURLE_OK;

done:
Curl_safefree(domain);
Curl_safefree(ntlm_auth_alloc);
free(domain);
free(ntlm_auth_alloc);
return CURLE_REMOTE_ACCESS_DENIED;
}

Expand Down Expand Up @@ -389,12 +389,12 @@ CURLcode Curl_output_ntlm_wb(struct connectdata *conn,
if(res)
return res;

Curl_safefree(*allocuserpwd);
free(*allocuserpwd);
*allocuserpwd = aprintf("%sAuthorization: %s\r\n",
proxy ? "Proxy-" : "",
conn->response_header);
DEBUG_OUT(fprintf(stderr, "**** Header %s\n ", *allocuserpwd));
Curl_safefree(conn->response_header);
free(conn->response_header);
conn->response_header = NULL;
break;
case NTLMSTATE_TYPE2:
Expand All @@ -407,7 +407,7 @@ CURLcode Curl_output_ntlm_wb(struct connectdata *conn,
if(res)
return res;

Curl_safefree(*allocuserpwd);
free(*allocuserpwd);
*allocuserpwd = aprintf("%sAuthorization: %s\r\n",
proxy ? "Proxy-" : "",
conn->response_header);
Expand Down
44 changes: 22 additions & 22 deletions lib/curl_sasl.c
Expand Up @@ -251,7 +251,7 @@ static CURLcode sasl_digest_get_qop_values(const char *options, int *value)
token = strtok_r(NULL, ",", &tok_buf);
}

Curl_safefree(tmp);
free(tmp);

return CURLE_OK;
}
Expand Down Expand Up @@ -324,7 +324,7 @@ static CURLcode sasl_create_plain_message(struct SessionHandle *data,
/* Base64 encode the reply */
result = Curl_base64_encode(data, plainauth, 2 * ulen + plen + 2, outptr,
outlen);
Curl_safefree(plainauth);
free(plainauth);
return result;
}

Expand Down Expand Up @@ -481,7 +481,7 @@ static CURLcode sasl_create_cram_md5_message(struct SessionHandle *data,
/* Base64 encode the response */
result = Curl_base64_encode(data, response, 0, outptr, outlen);

Curl_safefree(response);
free(response);

return result;
}
Expand Down Expand Up @@ -531,7 +531,7 @@ static CURLcode sasl_decode_digest_md5_message(const char *chlg64,

/* Retrieve nonce string from the challenge */
if(!sasl_digest_get_key_value((char *)chlg, "nonce=\"", nonce, nlen, '\"')) {
Curl_safefree(chlg);
free(chlg);
return CURLE_BAD_CONTENT_ENCODING;
}

Expand All @@ -543,17 +543,17 @@ static CURLcode sasl_decode_digest_md5_message(const char *chlg64,

/* Retrieve algorithm string from the challenge */
if(!sasl_digest_get_key_value((char *)chlg, "algorithm=", alg, alen, ',')) {
Curl_safefree(chlg);
free(chlg);
return CURLE_BAD_CONTENT_ENCODING;
}

/* Retrieve qop-options string from the challenge */
if(!sasl_digest_get_key_value((char *)chlg, "qop=\"", qop, qlen, '\"')) {
Curl_safefree(chlg);
free(chlg);
return CURLE_BAD_CONTENT_ENCODING;
}

Curl_safefree(chlg);
free(chlg);

return CURLE_OK;
}
Expand Down Expand Up @@ -675,7 +675,7 @@ CURLcode Curl_sasl_create_digest_md5_message(struct SessionHandle *data,
/* Calculate H(A2) */
ctxt = Curl_MD5_init(Curl_DIGEST_MD5);
if(!ctxt) {
Curl_safefree(spn);
free(spn);

return CURLE_OUT_OF_MEMORY;
}
Expand All @@ -693,7 +693,7 @@ CURLcode Curl_sasl_create_digest_md5_message(struct SessionHandle *data,
/* Now calculate the response hash */
ctxt = Curl_MD5_init(Curl_DIGEST_MD5);
if(!ctxt) {
Curl_safefree(spn);
free(spn);

return CURLE_OUT_OF_MEMORY;
}
Expand Down Expand Up @@ -726,14 +726,14 @@ CURLcode Curl_sasl_create_digest_md5_message(struct SessionHandle *data,
"qop=%s",
userp, realm, nonce,
cnonce, nonceCount, spn, resp_hash_hex, qop);
Curl_safefree(spn);
free(spn);
if(!response)
return CURLE_OUT_OF_MEMORY;

/* Base64 encode the response */
result = Curl_base64_encode(data, response, 0, outptr, outlen);

Curl_safefree(response);
free(response);

return result;
}
Expand Down Expand Up @@ -947,7 +947,7 @@ CURLcode Curl_sasl_create_digest_http_message(struct SessionHandle *data,

CURL_OUTPUT_DIGEST_CONV(data, md5this); /* convert on non-ASCII machines */
Curl_md5it(md5buf, md5this);
Curl_safefree(md5this);
free(md5this);
sasl_digest_md5_to_ascii(md5buf, ha1);

if(digest->algo == CURLDIGESTALGO_MD5SESS) {
Expand All @@ -958,7 +958,7 @@ CURLcode Curl_sasl_create_digest_http_message(struct SessionHandle *data,

CURL_OUTPUT_DIGEST_CONV(data, tmp); /* convert on non-ASCII machines */
Curl_md5it(md5buf, (unsigned char *)tmp);
Curl_safefree(tmp);
free(tmp);
sasl_digest_md5_to_ascii(md5buf, ha1);
}

Expand All @@ -982,7 +982,7 @@ CURLcode Curl_sasl_create_digest_http_message(struct SessionHandle *data,
TODO: replace md5 of empty string with entity-body for PUT/POST */
unsigned char *md5this2 = (unsigned char *)
aprintf("%s:%s", md5this, "d41d8cd98f00b204e9800998ecf8427e");
Curl_safefree(md5this);
free(md5this);
md5this = md5this2;
}

Expand All @@ -991,7 +991,7 @@ CURLcode Curl_sasl_create_digest_http_message(struct SessionHandle *data,

CURL_OUTPUT_DIGEST_CONV(data, md5this); /* convert on non-ASCII machines */
Curl_md5it(md5buf, md5this);
Curl_safefree(md5this);
free(md5this);
sasl_digest_md5_to_ascii(md5buf, ha2);

if(digest->qop) {
Expand All @@ -1015,7 +1015,7 @@ CURLcode Curl_sasl_create_digest_http_message(struct SessionHandle *data,

CURL_OUTPUT_DIGEST_CONV(data, md5this); /* convert on non-ASCII machines */
Curl_md5it(md5buf, md5this);
Curl_safefree(md5this);
free(md5this);
sasl_digest_md5_to_ascii(md5buf, request_digest);

/* for test case 64 (snooped from a Mozilla 1.3a request)
Expand Down Expand Up @@ -1070,7 +1070,7 @@ CURLcode Curl_sasl_create_digest_http_message(struct SessionHandle *data,
uripath,
request_digest);
}
Curl_safefree(userp_quoted);
free(userp_quoted);
if(!response)
return CURLE_OUT_OF_MEMORY;

Expand Down Expand Up @@ -1183,7 +1183,7 @@ static CURLcode sasl_create_xoauth2_message(struct SessionHandle *data,
/* Base64 encode the reply */
result = Curl_base64_encode(data, xoauth, strlen(xoauth), outptr, outlen);

Curl_safefree(xoauth);
free(xoauth);

return result;
}
Expand Down Expand Up @@ -1474,7 +1474,7 @@ CURLcode Curl_sasl_start(struct SASL *sasl, struct connectdata *conn,
if(!result) {
if(resp && sasl->params->maxirlen &&
strlen(mech) + len > sasl->params->maxirlen) {
Curl_safefree(resp);
free(resp);
resp = NULL;
}

Expand All @@ -1487,7 +1487,7 @@ CURLcode Curl_sasl_start(struct SASL *sasl, struct connectdata *conn,
}
}

Curl_safefree(resp);
free(resp);

return result;
}
Expand Down Expand Up @@ -1553,7 +1553,7 @@ CURLcode Curl_sasl_continue(struct SASL *sasl, struct connectdata *conn,
if(!result)
result = sasl_create_cram_md5_message(data, chlg, conn->user,
conn->passwd, &resp, &len);
Curl_safefree(chlg);
free(chlg);
break;
case SASL_DIGESTMD5:
sasl->params->getmessage(data->state.buffer, &serverdata);
Expand Down Expand Up @@ -1659,7 +1659,7 @@ CURLcode Curl_sasl_continue(struct SASL *sasl, struct connectdata *conn,
break;
}

Curl_safefree(resp);
free(resp);

state(sasl, conn, newstate);

Expand Down

2 comments on commit 0f4a03c

@jay
Copy link
Member

@jay jay commented on 0f4a03c Mar 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Daniel going through this I saw a lot of places (41) where it just isn't clear that the variable is going to be reassigned to or reach scope end in a reasonable amount of lines. I made a partial revert jay/curl@2eaf855 to cover those cases, if you're interested.

It strikes me as difficult to manage free without a pattern that is recognizable. It could be worth a discussion on replacing all free with Curl_safefree or always requiring variable reassignment or scope end right after to avoid a use-after-free. The discussion may have already happened, it sounds familiar..

@bagder
Copy link
Member Author

@bagder bagder commented on 0f4a03c Mar 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but for most of these cases I actually checked if the variable was referenced any more within the scope and if not, then not assigning it after the free() is not a problem. I'm not convinced assigning pointers to NULL "just in case" is very productive. Accessing such a pointer after the free() could be equally wrong even if it was assigned NULL (although it won't crash)! Our own memory tracking system as well as valgrind will help us catch any such bad accesses but only if it is not assigned to NULL because then it'll be hidden.

Please sign in to comment.