* [PATCH] Check return value of ftruncate call in http.c. Remove possible mem leak @ 2009-08-10 13:42 Jeff Lasslett 2009-08-10 15:47 ` Tay Ray Chuan 0 siblings, 1 reply; 6+ messages in thread From: Jeff Lasslett @ 2009-08-10 13:42 UTC (permalink / raw) To: git, gitster; +Cc: 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Check return value of ftruncate call in http.c. Remove possible mem leak 2009-08-10 13:42 [PATCH] Check return value of ftruncate call in http.c. Remove possible mem leak Jeff Lasslett @ 2009-08-10 15:47 ` Tay Ray Chuan 2009-08-10 15:55 ` [PATCH (resend) 1/3] http.c: free preq when aborting Tay Ray Chuan 0 siblings, 1 reply; 6+ messages in thread From: Tay Ray Chuan @ 2009-08-10 15:47 UTC (permalink / raw) To: Jeff Lasslett; +Cc: git, gitster 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH (resend) 1/3] http.c: free preq when aborting 2009-08-10 15:47 ` Tay Ray Chuan @ 2009-08-10 15:55 ` Tay Ray Chuan 2009-08-10 15:59 ` [PATCH (resend) 2/3] http.c: replace usage of temporary variable for urls Tay Ray Chuan 0 siblings, 1 reply; 6+ messages in thread From: Tay Ray Chuan @ 2009-08-10 15:55 UTC (permalink / raw) To: git, gitster; +Cc: Jeff Lasslett Free preq in new_http_pack_request when aborting. preq was allocated before jumping to the 'abort' label so this is safe. Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- http.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/http.c b/http.c index a2720d5..cfe32f5 100644 --- a/http.c +++ b/http.c @@ -1059,6 +1059,7 @@ struct http_pack_request *new_http_pack_request( abort: free(filename); + free(preq); return NULL; } -- 1.6.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH (resend) 2/3] http.c: replace usage of temporary variable for urls 2009-08-10 15:55 ` [PATCH (resend) 1/3] http.c: free preq when aborting Tay Ray Chuan @ 2009-08-10 15:59 ` Tay Ray Chuan 2009-08-10 16:05 ` [PATCH (resend) 3/3] Check return value of ftruncate call in http.c Tay Ray Chuan 0 siblings, 1 reply; 6+ messages in thread From: Tay Ray Chuan @ 2009-08-10 15:59 UTC (permalink / raw) To: git, gitster; +Cc: Jeff Lasslett Use preq->url in new_http_pack_request and freq->url in new_http_object_request when calling curl_setopt(CURLOPT_URL), instead of using an intermediate variable, 'url'. Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- http.c | 15 ++++++--------- 1 files changed, 6 insertions(+), 9 deletions(-) diff --git a/http.c b/http.c index cfe32f5..98f9707 100644 --- a/http.c +++ b/http.c @@ -1004,7 +1004,6 @@ int finish_http_pack_request(struct http_pack_request *preq) struct http_pack_request *new_http_pack_request( struct packed_git *target, const char *base_url) { - char *url; char *filename; long prev_posn = 0; char range[RANGE_HEADER_SIZE]; @@ -1018,8 +1017,7 @@ struct http_pack_request *new_http_pack_request( end_url_with_slash(&buf, base_url); strbuf_addf(&buf, "objects/pack/pack-%s.pack", sha1_to_hex(target->sha1)); - url = strbuf_detach(&buf, NULL); - preq->url = xstrdup(url); + preq->url = strbuf_detach(&buf, NULL); filename = sha1_pack_name(target->sha1); snprintf(preq->filename, sizeof(preq->filename), "%s", filename); @@ -1035,7 +1033,7 @@ struct http_pack_request *new_http_pack_request( preq->slot->local = preq->packfile; curl_easy_setopt(preq->slot->curl, CURLOPT_FILE, preq->packfile); curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite); - curl_easy_setopt(preq->slot->curl, CURLOPT_URL, url); + curl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url); curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); @@ -1059,6 +1057,7 @@ struct http_pack_request *new_http_pack_request( abort: free(filename); + free(preq->url); free(preq); return NULL; } @@ -1099,7 +1098,6 @@ 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; int prevlocal; unsigned char prev_buf[PREV_BUF_SIZE]; ssize_t prev_read = 0; @@ -1153,8 +1151,7 @@ struct http_object_request *new_http_object_request(const char *base_url, git_SHA1_Init(&freq->c); - url = get_remote_object_url(base_url, hex, 0); - freq->url = xstrdup(url); + freq->url = get_remote_object_url(base_url, hex, 0); /* * If a previous temp file is present, process what was already @@ -1199,7 +1196,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); /* @@ -1219,9 +1216,9 @@ struct http_object_request *new_http_object_request(const char *base_url, return freq; - free(url); abort: free(filename); + free(freq->url); free(freq); return NULL; } -- 1.6.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH (resend) 3/3] Check return value of ftruncate call in http.c 2009-08-10 15:59 ` [PATCH (resend) 2/3] http.c: replace usage of temporary variable for urls Tay Ray Chuan @ 2009-08-10 16:05 ` Tay Ray Chuan 2009-08-11 5:13 ` Junio C Hamano 0 siblings, 1 reply; 6+ messages in thread From: Tay Ray Chuan @ 2009-08-10 16:05 UTC (permalink / raw) To: git, gitster; +Cc: Jeff Lasslett From: Jeff Lasslett <jeff.lasslett@gmail.com> In new_http_object_request(), check ftruncate() call return value and handle possible errors. Signed-off-by: Jeff Lasslett <jeff.lasslett@gmail.com> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- Jeff, I took out all the url-specific cleanups and expanded their scope (see earlier patches). You can safely focus on ftruncate() and do an abort there. :) http.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/http.c b/http.c index 98f9707..14d5357 100644 --- a/http.c +++ b/http.c @@ -1187,7 +1187,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; + } } } -- 1.6.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH (resend) 3/3] Check return value of ftruncate call in http.c 2009-08-10 16:05 ` [PATCH (resend) 3/3] Check return value of ftruncate call in http.c Tay Ray Chuan @ 2009-08-11 5:13 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2009-08-11 5:13 UTC (permalink / raw) To: Tay Ray Chuan; +Cc: git, Jeff Lasslett Thanks; all three patches queued. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-08-11 12:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-08-10 13:42 [PATCH] Check return value of ftruncate call in http.c. Remove possible mem leak Jeff Lasslett 2009-08-10 15:47 ` Tay Ray Chuan 2009-08-10 15:55 ` [PATCH (resend) 1/3] http.c: free preq when aborting Tay Ray Chuan 2009-08-10 15:59 ` [PATCH (resend) 2/3] http.c: replace usage of temporary variable for urls Tay Ray Chuan 2009-08-10 16:05 ` [PATCH (resend) 3/3] Check return value of ftruncate call in http.c Tay Ray Chuan 2009-08-11 5:13 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).