[PATCH] Check return value of ftruncate call in http.c. Remove possible mem leak

[PATCH] Check return value of ftruncate call in http.c. Remove possible mem leak

[Jeff Lasslett]

In new_http_object_request(), check ftruncate() call return value and
handle possible errors.  Remove possible leak of mem pointed to by url.

Signed-off-by: Jeff Lasslett <jeff.lasslett@gmail.com>
---
 http.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/http.c b/http.c
index a2720d5..34a3a35 100644
--- a/http.c
+++ b/http.c
@@ -1098,7 +1098,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
 	char *hex = sha1_to_hex(sha1);
 	char *filename;
 	char prevfile[PATH_MAX];
-	char *url;
+	char *url = NULL;
 	int prevlocal;
 	unsigned char prev_buf[PREV_BUF_SIZE];
 	ssize_t prev_read = 0;
@@ -1189,7 +1189,11 @@ struct http_object_request *new_http_object_request(const char *base_url,
 		if (prev_posn>0) {
 			prev_posn = 0;
 			lseek(freq->localfile, 0, SEEK_SET);
-			ftruncate(freq->localfile, 0);
+			if (ftruncate(freq->localfile, 0) < 0) {
+				error("Couldn't truncate temporary file %s for %s: %s",
+					  freq->tmpfile, freq->filename, strerror(errno));
+				goto abort;
+			}
 		}
 	}
 
@@ -1216,10 +1220,12 @@ struct http_object_request *new_http_object_request(const char *base_url,
 				 CURLOPT_HTTPHEADER, range_header);
 	}
 
+	free(url);
 	return freq;
 
-	free(url);
 abort:
+	free(url);
+	free(freq->url);
 	free(filename);
 	free(freq);
 	return NULL;
-- 
1.6.4.59.g4d590.dirty

Re: [PATCH] Check return value of ftruncate call in http.c. Remove possible mem leak

[Junio C Hamano]

Jeff Lasslett <jeff.lasslett@gmail.com> writes:

> In new_http_object_request(), check ftruncate() call return value and
> handle possible errors.  Remove possible leak of mem pointed to by url.

Are you assuming that everybody is running libcurl 7.17.0 or later?
Before that version strings given to easy_setopt() were not copied and you
needed to keep them around.

I think freeing url is fine but you would probably need to then give
freq->url to easy_setopt(CURLOPT_URL) if you wanted to do so.

[PATCH] Check return value of ftruncate call in http.c. Remove possible mem leak

[Jeff Lasslett]

In new_http_object_request(), check ftruncate() call return value and
handle possible errors.  Remove possible leak of mem pointed to by url.

Signed-off-by: Jeff Lasslett <jeff.lasslett@gmail.com>
---
 http.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/http.c b/http.c
index a2720d5..e8317e1 100644
--- a/http.c
+++ b/http.c
@@ -1098,7 +1098,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
 	char *hex = sha1_to_hex(sha1);
 	char *filename;
 	char prevfile[PATH_MAX];
-	char *url;
+	char *url = NULL;
 	int prevlocal;
 	unsigned char prev_buf[PREV_BUF_SIZE];
 	ssize_t prev_read = 0;
@@ -1189,7 +1189,11 @@ struct http_object_request *new_http_object_request(const char *base_url,
 		if (prev_posn>0) {
 			prev_posn = 0;
 			lseek(freq->localfile, 0, SEEK_SET);
-			ftruncate(freq->localfile, 0);
+			if (ftruncate(freq->localfile, 0) < 0) {
+				error("Couldn't truncate temporary file %s for %s: %s",
+					  freq->tmpfile, freq->filename, strerror(errno));
+				goto abort;
+			}
 		}
 	}
 
@@ -1198,7 +1202,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
 	curl_easy_setopt(freq->slot->curl, CURLOPT_FILE, freq);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);
-	curl_easy_setopt(freq->slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_HTTPHEADER, no_pragma_header);
 
 	/*
@@ -1216,10 +1220,12 @@ struct http_object_request *new_http_object_request(const char *base_url,
 				 CURLOPT_HTTPHEADER, range_header);
 	}
 
+	free(url);
 	return freq;
 
-	free(url);
 abort:
+	free(url);
+	free(freq->url);
 	free(filename);
 	free(freq);
 	return NULL;
-- 
1.6.4.59.g4d590.dirty

Re: [PATCH] Check return value of ftruncate call in http.c. Remove possible mem leak

[Tay Ray Chuan]

Hi,

On Mon, Aug 10, 2009 at 9:42 PM, Jeff Lasslett<jeff.lasslett@gmail.com> wrote:
> @@ -1216,10 +1220,12 @@ struct http_object_request *new_http_object_request(const char *base_url,
>                                 CURLOPT_HTTPHEADER, range_header);
>        }
>
> +       free(url);
>        return freq;
>
> -       free(url);

Freeing after the return didn't make sense. Thanks for spotting this.

On a side note, you could simply do away with the 'url' variable,
since you're going to pass freq->url to curl_setopt.

-- 
Cheers,
Ray Chuan