[PATCH 1/8] ui-tree: fix resource leak: free before return

[PATCH 1/8] ui-tree: fix resource leak: free before return

[list]

From: Christian Hesse <mail at eworm.de>

Coverity-id: 13938
Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-tree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui-tree.c b/ui-tree.c
index 1b310d5..d71503d 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -143,6 +143,7 @@ static int ls_item(const unsigned char *sha1, struct strbuf *base,
 			htmlf("<tr><td colspan='3'>Bad object: %s %s</td></tr>",
 			      name,
 			      sha1_to_hex(sha1));
+			free(name);
 			return 0;
 		}
 	}
-- 
2.6.1

[PATCH 2/8] ui-ssdiff: fix resource leak: free allocation from cgit_fileurl

[list]

From: Christian Hesse <mail at eworm.de>

Coverity-id: 13929
Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-ssdiff.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ui-ssdiff.c b/ui-ssdiff.c
index 2146c71..d183d40 100644
--- a/ui-ssdiff.c
+++ b/ui-ssdiff.c
@@ -230,11 +230,13 @@ static void print_ssdiff_line(char *class,
 		struct diff_filespec *old_file = cgit_get_current_old_file();
 		char *lineno_str = fmt("n%d", old_line_no);
 		char *id_str = fmt("id=%s#%s", is_null_sha1(old_file->sha1)?"HEAD":sha1_to_hex(old_rev_sha1), lineno_str);
+		char *fileurl = cgit_fileurl(ctx.repo->url, "tree", old_file->path, id_str);
 		html("<td class='lineno'><a href='");
-		html(cgit_fileurl(ctx.repo->url, "tree", old_file->path, id_str));
+		html(fileurl);
 		htmlf("' id='%s'>%s</a>", lineno_str, lineno_str + 1);
 		html("</td>");
 		htmlf("<td class='%s'>", class);
+		free(fileurl);
 	} else if (old_line)
 		htmlf("<td class='lineno'></td><td class='%s'>", class);
 	else
@@ -251,11 +253,13 @@ static void print_ssdiff_line(char *class,
 		struct diff_filespec *new_file = cgit_get_current_new_file();
 		char *lineno_str = fmt("n%d", new_line_no);
 		char *id_str = fmt("id=%s#%s", is_null_sha1(new_file->sha1)?"HEAD":sha1_to_hex(new_rev_sha1), lineno_str);
+		char *fileurl = cgit_fileurl(ctx.repo->url, "tree", new_file->path, id_str);
 		html("<td class='lineno'><a href='");
-		html(cgit_fileurl(ctx.repo->url, "tree", new_file->path, id_str));
+		html(fileurl);
 		htmlf("' id='%s'>%s</a>", lineno_str, lineno_str + 1);
 		html("</td>");
 		htmlf("<td class='%s'>", class);
+		free(fileurl);
 	} else if (new_line)
 		htmlf("<td class='lineno'></td><td class='%s'>", class);
 	else
-- 
2.6.1

[PATCH 2/8] ui-ssdiff: fix resource leak: free allocation from cgit_fileurl

[Jason]

Merged.

[PATCH 3/8] ui-shared: fix resource leak: free allocation from cgit_fileurl

[list]

From: Christian Hesse <mail at eworm.de>

Coverity-id: 13918
Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-shared.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/ui-shared.c b/ui-shared.c
index 89c4897..d9f8258 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -746,16 +746,19 @@ void cgit_print_docstart(void)
 		html("'/>\n");
 	}
 	if (host && ctx.repo && ctx.qry.head) {
+		char *fileurl;
 		struct strbuf sb = STRBUF_INIT;
 		strbuf_addf(&sb, "h=%s", ctx.qry.head);
 
 		html("<link rel='alternate' title='Atom feed' href='");
 		html(cgit_httpscheme());
 		html_attr(cgit_hosturl());
-		html_attr(cgit_fileurl(ctx.repo->url, "atom", ctx.qry.vpath,
-				       sb.buf));
+		fileurl = cgit_fileurl(ctx.repo->url, "atom", ctx.qry.vpath,
+				       sb.buf);
+		html_attr(fileurl);
 		html("' type='application/atom+xml'/>\n");
 		strbuf_release(&sb);
+		free(fileurl);
 	}
 	if (ctx.repo)
 		cgit_add_clone_urls(print_rel_vcs_link);
@@ -997,9 +1000,12 @@ void cgit_print_pageheader(void)
 					ctx.qry.head, ctx.qry.vpath);
 		html("</td><td class='form'>");
 		html("<form class='right' method='get' action='");
-		if (ctx.cfg.virtual_root)
-			html_url_path(cgit_fileurl(ctx.qry.repo, "log",
-						   ctx.qry.vpath, NULL));
+		if (ctx.cfg.virtual_root) {
+			char *fileurl = cgit_fileurl(ctx.qry.repo, "log",
+						   ctx.qry.vpath, NULL);
+			html_url_path(fileurl);
+			free(fileurl);
+		}
 		html("'>\n");
 		cgit_add_hidden_formfields(1, 0, "log");
 		html("<select name='qt'>\n");
-- 
2.6.1

[PATCH 4/8] ui-shared: return value of cgit_currenturl is not const

[list]

From: Christian Hesse <mail at eworm.de>

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-shared.c | 4 ++--
 ui-shared.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui-shared.c b/ui-shared.c
index d9f8258..b739675 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -65,10 +65,10 @@ const char *cgit_hosturl(void)
 	return fmtalloc("%s:%s", ctx.env.server_name, ctx.env.server_port);
 }
 
-const char *cgit_currenturl(void)
+char *cgit_currenturl(void)
 {
 	if (!ctx.qry.url)
-		return cgit_rooturl();
+		return xstrdup(cgit_rooturl());
 	const char *root = cgit_rooturl();
 	size_t len = strlen(root);
 	if (len && root[len - 1] == '/')
diff --git a/ui-shared.h b/ui-shared.h
index 246678b..4121416 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -4,7 +4,7 @@
 extern const char *cgit_httpscheme();
 extern const char *cgit_hosturl();
 extern const char *cgit_rooturl();
-extern const char *cgit_currenturl();
+extern char *cgit_currenturl();
 extern const char *cgit_loginurl();
 extern char *cgit_repourl(const char *reponame);
 extern char *cgit_fileurl(const char *reponame, const char *pagename,
-- 
2.6.1

[PATCH 4/8] ui-shared: return value of cgit_currenturl is not const

[Jason]

This is a good normalization to make. I vaguely remember when adding
this function that I was thinking, "hm, that's not right, but it's not
like we care about memory leaks anyway so...". Thanks for fixing my
laziness.

Merged.

[PATCH 5/8] ui-shared: fix resource leak: free allocation from cgit_currenturl

[list]

From: Christian Hesse <mail at eworm.de>

Coverity-id: 13927
Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-shared.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ui-shared.c b/ui-shared.c
index b739675..f6d38de 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -157,8 +157,11 @@ static void site_url(const char *page, const char *search, const char *sort, int
 
 	if (always_root || page)
 		html_attr(cgit_rooturl());
-	else
-		html_attr(cgit_currenturl());
+	else {
+		char *currenturl = cgit_currenturl();
+		html_attr(currenturl);
+		free(currenturl);
+	}
 
 	if (page) {
 		htmlf("?p=%s", page);
@@ -1020,19 +1023,21 @@ void cgit_print_pageheader(void)
 		html("<input type='submit' value='search'/>\n");
 		html("</form>\n");
 	} else if (ctx.env.authenticated) {
+		char *currenturl = cgit_currenturl();
 		site_link(NULL, "index", NULL, hc("repolist"), NULL, NULL, 0, 1);
 		if (ctx.cfg.root_readme)
 			site_link("about", "about", NULL, hc("about"),
 				  NULL, NULL, 0, 1);
 		html("</td><td class='form'>");
 		html("<form method='get' action='");
-		html_attr(cgit_currenturl());
+		html_attr(currenturl);
 		html("'>\n");
 		html("<input type='text' name='q' size='10' value='");
 		html_attr(ctx.qry.search);
 		html("'/>\n");
 		html("<input type='submit' value='search'/>\n");
 		html("</form>");
+		free(currenturl);
 	}
 	html("</td></tr></table>\n");
 	if (ctx.env.authenticated && ctx.qry.vpath) {
-- 
2.6.1

[PATCH 5/8] ui-shared: fix resource leak: free allocation from cgit_currenturl

[Jason]

Merged.

[PATCH 6/8] cmd: fix resource leak: free allocation from cgit_currenturl and fmtalloc

[list]

From: Christian Hesse <mail at eworm.de>

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 cmd.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/cmd.c b/cmd.c
index c991092..3093c62 100644
--- a/cmd.c
+++ b/cmd.c
@@ -41,9 +41,13 @@ static void about_fn(void)
 	if (ctx.repo) {
 		if (!ctx.qry.path &&
 		    ctx.qry.url[strlen(ctx.qry.url) - 1] != '/' &&
-		    ctx.env.path_info[strlen(ctx.env.path_info) - 1] != '/')
-			cgit_redirect(fmtalloc("%s/", cgit_currenturl()), true);
-		else
+		    ctx.env.path_info[strlen(ctx.env.path_info) - 1] != '/') {
+			char *currenturl = cgit_currenturl();
+			char *redirect = fmtalloc("%s/", currenturl);
+			cgit_redirect(redirect, true);
+			free(currenturl);
+			free(redirect);
+		} else
 			cgit_print_repo_readme(ctx.qry.path);
 	} else
 		cgit_print_site_readme();
-- 
2.6.1

[PATCH 6/8] cmd: fix resource leak: free allocation from cgit_currenturl and fmtalloc

[Jason]

No coverity-id on this commit. Did you find this one yourself?

In either case, merged. Thanks.

[PATCH 6/8] cmd: fix resource leak: free allocation from cgit_currenturl and fmtalloc

[list]

"Jason A. Donenfeld" <Jason at zx2c4.com> on Fri, 2015/10/09 14:03:
> No coverity-id on this commit. Did you find this one yourself?

Yes. No idea why coverity did not see this...
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Chris           get my mail address:    */=0;b=c[a++];)
putchar(b-1/(/*               gcc -o sig sig.c && ./sig    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20151009/87227c8c/attachment.asc>

[PATCH 6/8] cmd: fix resource leak: free allocation from cgit_currenturl and fmtalloc

[mailings]

I have the experience that it sometimes misses things that are then 
found in the next build.
Or that it flags a whole load of lines without issues.
It doesn't happen a lot though. Sometimes.

Let's call it temperamental...


On 09/10/15 14:08, Christian Hesse wrote:
> "Jason A. Donenfeld" <Jason at zx2c4.com> on Fri, 2015/10/09 14:03:
>> No coverity-id on this commit. Did you find this one yourself?
>
> Yes. No idea why coverity did not see this...
>
>
>
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
>

-- 
Ferry Huberts

[PATCH 6/8] cmd: fix resource leak: free allocation from cgit_currenturl and fmtalloc

[list]

Ferry Huberts <mailings at hupie.com> on Fri, 2015/10/09 14:13:
> I have the experience that it sometimes misses things that are then 
> found in the next build.
> Or that it flags a whole load of lines without issues.
> It doesn't happen a lot though. Sometimes.
> 
> Let's call it temperamental...

Uh, it lives and has its own mind... :-P
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Chris           get my mail address:    */=0;b=c[a++];)
putchar(b-1/(/*               gcc -o sig sig.c && ./sig    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20151009/37cd6f33/attachment-0001.asc>

[PATCH 6/8] cmd: fix resource leak: free allocation from cgit_currenturl and fmtalloc

[Jason]

My guess is the fact that sometimes that function returned a const
values and other times an allocated value tripped it up. It's flow
graph can only be so big...

[PATCH 7/8] ui-shared: return value of cgit_hosturl is not const

[list]

From: Christian Hesse <mail at eworm.de>

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-shared.c | 6 +++---
 ui-shared.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui-shared.c b/ui-shared.c
index f6d38de..09c34fd 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -54,14 +54,14 @@ const char *cgit_httpscheme(void)
 		return "http://";
 }
 
-const char *cgit_hosturl(void)
+char *cgit_hosturl(void)
 {
 	if (ctx.env.http_host)
-		return ctx.env.http_host;
+		return xstrdup(ctx.env.http_host);
 	if (!ctx.env.server_name)
 		return NULL;
 	if (!ctx.env.server_port || atoi(ctx.env.server_port) == 80)
-		return ctx.env.server_name;
+		return xstrdup(ctx.env.server_name);
 	return fmtalloc("%s:%s", ctx.env.server_name, ctx.env.server_port);
 }
 
diff --git a/ui-shared.h b/ui-shared.h
index 4121416..474e0c5 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -2,7 +2,7 @@
 #define UI_SHARED_H
 
 extern const char *cgit_httpscheme();
-extern const char *cgit_hosturl();
+extern char *cgit_hosturl();
 extern const char *cgit_rooturl();
 extern char *cgit_currenturl();
 extern const char *cgit_loginurl();
-- 
2.6.1

[PATCH 7/8] ui-shared: return value of cgit_hosturl is not const

[Jason]

Again, nice normalization here. Merged, thanks.

[PATCH 8/8] ui-shared: fix resource leak: free allocation from cgit_hosturl

[list]

From: Christian Hesse <mail at eworm.de>

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 ui-shared.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ui-shared.c b/ui-shared.c
index 09c34fd..c04f380 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -730,7 +730,7 @@ void cgit_print_docstart(void)
 		return;
 	}
 
-	const char *host = cgit_hosturl();
+	char *host = cgit_hosturl();
 	html(cgit_doctype);
 	html("<html xmlns='http://www.w3.org/1999/xhtml' xml:lang='en' lang='en'>\n");
 	html("<head>\n");
@@ -755,7 +755,7 @@ void cgit_print_docstart(void)
 
 		html("<link rel='alternate' title='Atom feed' href='");
 		html(cgit_httpscheme());
-		html_attr(cgit_hosturl());
+		html_attr(host);
 		fileurl = cgit_fileurl(ctx.repo->url, "atom", ctx.qry.vpath,
 				       sb.buf);
 		html_attr(fileurl);
@@ -771,6 +771,7 @@ void cgit_print_docstart(void)
 	html("<body>\n");
 	if (ctx.cfg.header)
 		html_include(ctx.cfg.header);
+	free(host);
 }
 
 void cgit_print_docend(void)
-- 
2.6.1

[PATCH 8/8] ui-shared: fix resource leak: free allocation from cgit_hosturl

[Jason]

Merged, thanks.

Also - no coverity-id on this one too.

[PATCH 8/8] ui-shared: fix resource leak: free allocation from cgit_hosturl

[list]

"Jason A. Donenfeld" <Jason at zx2c4.com> on Fri, 2015/10/09 14:04:
> Merged, thanks.
> 
> Also - no coverity-id on this one too.

After fixing the function return type I updated the variable type on line
730 - which converity did not mark - and fixed remaining issues. Coverity id
for the last part is 13920.
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Chris           get my mail address:    */=0;b=c[a++];)
putchar(b-1/(/*               gcc -o sig sig.c && ./sig    */b/42*2-3)*42);}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20151009/d00e8c48/attachment.asc>

[PATCH 1/8] ui-tree: fix resource leak: free before return

[Jason]

Merged.