[PATCH 1/2] move get_mimetype_from_file() to shared

[PATCH 1/2] move get_mimetype_from_file() to shared

[list]

From: Christian Hesse <mail at eworm.de>

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

diff --git a/cgit.h b/cgit.h
index f327627..0f1e186 100644
--- a/cgit.h
+++ b/cgit.h
@@ -391,4 +391,6 @@ extern int readfile(const char *path, char **buf, size_t *size);
 
 extern char *expand_macros(const char *txt);
 
+extern char *get_mimetype_from_file(const char *filename, const char *ext);
+
 #endif /* CGIT_H */
diff --git a/shared.c b/shared.c
index 3bd2a9e..5a000a6 100644
--- a/shared.c
+++ b/shared.c
@@ -560,3 +560,44 @@ char *expand_macros(const char *txt)
 	}
 	return result;
 }
+
+char *get_mimetype_from_file(const char *filename, const char *ext)
+{
+	static const char *delimiters;
+	char *result;
+	FILE *fd;
+	char line[1024];
+	char *mimetype;
+	char *token;
+
+	if (!filename)
+		return NULL;
+
+	fd = fopen(filename, "r");
+	if (!fd)
+		return NULL;
+
+	delimiters = " \t\r\n";
+	result = NULL;
+
+	/* loop over all lines in the file */
+	while (!result && fgets(line, sizeof(line), fd)) {
+		mimetype = strtok(line, delimiters);
+
+		/* skip empty lines and comment lines */
+		if (!mimetype || (mimetype[0] == '#'))
+			continue;
+
+		/* loop over all extensions of mimetype */
+		while ((token = strtok(NULL, delimiters))) {
+			if (!strcasecmp(ext, token)) {
+				result = xstrdup(mimetype);
+				break;
+			}
+		}
+	}
+	fclose(fd);
+
+	return result;
+}
+
diff --git a/ui-plain.c b/ui-plain.c
index 0daa7bf..d68518e 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -16,46 +16,6 @@ struct walk_tree_context {
 	int match;
 };
 
-static char *get_mimetype_from_file(const char *filename, const char *ext)
-{
-	static const char *delimiters;
-	char *result;
-	FILE *fd;
-	char line[1024];
-	char *mimetype;
-	char *token;
-
-	if (!filename)
-		return NULL;
-
-	fd = fopen(filename, "r");
-	if (!fd)
-		return NULL;
-
-	delimiters = " \t\r\n";
-	result = NULL;
-
-	/* loop over all lines in the file */
-	while (!result && fgets(line, sizeof(line), fd)) {
-		mimetype = strtok(line, delimiters);
-
-		/* skip empty lines and comment lines */
-		if (!mimetype || (mimetype[0] == '#'))
-			continue;
-
-		/* loop over all extensions of mimetype */
-		while ((token = strtok(NULL, delimiters))) {
-			if (!strcasecmp(ext, token)) {
-				result = xstrdup(mimetype);
-				break;
-			}
-		}
-	}
-	fclose(fd);
-
-	return result;
-}
-
 static int print_object(const unsigned char *sha1, const char *path)
 {
 	enum object_type type;
-- 
2.5.0

[PATCH 2/2] about: send images plain

[list]

From: Christian Hesse <mail at eworm.de>

The about page used to display just fine, but images were broken: The
binary image data was embedded in html code.
Use cgit_print_plain() to send images in plain mode and make them
available on about page.

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

diff --git a/cmd.c b/cmd.c
index 20c80b0..d77ee63 100644
--- a/cmd.c
+++ b/cmd.c
@@ -43,8 +43,37 @@ static void about_fn(void)
 		    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
-			cgit_print_repo_readme(ctx.qry.path);
+		else {
+			char *ext = NULL;
+			int freemime = 0;
+			struct string_list_item *mime;
+			char * mimetype = NULL;
+
+			if (ctx.qry.path)
+				ext = strrchr(ctx.qry.path, '.');
+
+			if (ext && *(++ext)) {
+				mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
+				if (mime) {
+					mimetype = (char *)mime->util;
+				} else {
+					mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
+					if (mimetype)
+						freemime = 1;
+				}
+			}
+
+			if (mimetype && strncmp(mimetype, "image/", 6) == 0) {
+				ctx.page.mimetype = mimetype;
+				ctx.page.charset = NULL;
+				cgit_print_plain();
+			} else
+				cgit_print_repo_readme(ctx.qry.path);
+
+			/* If we allocated this, then casting away const is safe. */
+			if (freemime)
+				free(mimetype);
+		}
 	} else
 		cgit_print_site_readme();
 }
-- 
2.5.0

[PATCH 2/2] about: send images plain

[john]

On Fri, Aug 14, 2015 at 04:50:57PM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> The about page used to display just fine, but images were broken: The
> binary image data was embedded in html code.
> Use cgit_print_plain() to send images in plain mode and make them
> available on about page.
> 
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
>  cmd.c | 33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/cmd.c b/cmd.c
> index 20c80b0..d77ee63 100644
> --- a/cmd.c
> +++ b/cmd.c
> @@ -43,8 +43,37 @@ static void about_fn(void)
>  		    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
> -			cgit_print_repo_readme(ctx.qry.path);

The entire block below is probably better as an implementation detail of
cgit_print_repo_readme(), so that about_fn() remains relatively concise.

> +		else {
> +			char *ext = NULL;
> +			int freemime = 0;
> +			struct string_list_item *mime;
> +			char * mimetype = NULL;
> +

The section below is very similar to bits of ui-plain.c::print_object().
I wonder if ui-shared.c should have something like:

	int mimetype_for_filename(const char *filename, const char **mime, int *freemime);

and we can keep get_mimetype_from_file() as an implementation detail.

> +			if (ctx.qry.path)
> +				ext = strrchr(ctx.qry.path, '.');
> +
> +			if (ext && *(++ext)) {
> +				mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> +				if (mime) {
> +					mimetype = (char *)mime->util;
> +				} else {
> +					mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
> +					if (mimetype)
> +						freemime = 1;
> +				}
> +			}
> +
> +			if (mimetype && strncmp(mimetype, "image/", 6) == 0) {

In ui-plain.c we inspect the buffer to see if the file looks binary; I
wonder if that makes sense here (not that it's necessarily easy to do
so).

> +				ctx.page.mimetype = mimetype;
> +				ctx.page.charset = NULL;
> +				cgit_print_plain();
> +			} else
> +				cgit_print_repo_readme(ctx.qry.path);
> +
> +			/* If we allocated this, then casting away const is safe. */
> +			if (freemime)
> +				free(mimetype);
> +		}
>  	} else
>  		cgit_print_site_readme();
>  }
> -- 
> 2.5.0

[PATCH 2/3] about: send images plain

[list]

From: Christian Hesse <mail at eworm.de>

The about page used to display just fine, but images were broken: The
binary image data was embedded in html code.
Use cgit_print_plain() to send images in plain mode and make them
available on about page.

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

diff --git a/ui-summary.c b/ui-summary.c
index fb04dc3..99c9234 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -9,9 +9,10 @@
 #include "cgit.h"
 #include "ui-summary.h"
 #include "html.h"
+#include "ui-blob.h"
 #include "ui-log.h"
+#include "ui-plain.h"
 #include "ui-refs.h"
-#include "ui-blob.h"
 #include "ui-shared.h"
 
 static int urls;
@@ -100,8 +101,38 @@ static char* append_readme_path(const char *filename, const char *ref, const cha
 
 void cgit_print_repo_readme(char *path)
 {
-	char *filename, *ref;
+	char *ext = NULL, *filename, *ref, *mimetype = NULL;
 	int free_filename = 0;
+	int freemime = 0;
+	struct string_list_item *mime;
+
+	if (ctx.qry.path)
+		ext = strrchr(ctx.qry.path, '.');
+
+	if (ext && *(++ext)) {
+		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
+		if (mime) {
+			mimetype = (char *)mime->util;
+		} else {
+			mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
+			if (mimetype)
+				freemime = 1;
+		}
+	}
+
+	if (mimetype && strncmp(mimetype, "image/", 6) == 0) {
+		ctx.page.mimetype = mimetype;
+		ctx.page.charset = NULL;
+		cgit_print_plain();
+
+		if (freemime)
+			free(mimetype);
+
+		return;
+	}
+
+	if (freemime)
+		free(mimetype);
 
 	cgit_print_layout_start();
 	if (ctx.repo->readme.nr == 0)
-- 
2.5.0

[PATCH 3/3] move shared code to get_mimetype_from_file()

[list]

From: Christian Hesse <mail at eworm.de>

Signed-off-by: Christian Hesse <mail at eworm.de>
---
 cgit.h       |  2 +-
 shared.c     | 57 +++++++++++++++++++++++++++++++++------------------------
 ui-plain.c   | 25 ++++---------------------
 ui-summary.c | 28 ++++------------------------
 4 files changed, 42 insertions(+), 70 deletions(-)

diff --git a/cgit.h b/cgit.h
index 0f1e186..dfa6d0c 100644
--- a/cgit.h
+++ b/cgit.h
@@ -391,6 +391,6 @@ extern int readfile(const char *path, char **buf, size_t *size);
 
 extern char *expand_macros(const char *txt);
 
-extern char *get_mimetype_from_file(const char *filename, const char *ext);
+extern void get_mimetype_from_file(const char *qrypath, const char **mimetype);
 
 #endif /* CGIT_H */
diff --git a/shared.c b/shared.c
index 5a000a6..1f049e7 100644
--- a/shared.c
+++ b/shared.c
@@ -561,43 +561,52 @@ char *expand_macros(const char *txt)
 	return result;
 }
 
-char *get_mimetype_from_file(const char *filename, const char *ext)
+void get_mimetype_from_file(const char *qrypath, const char **mimetype)
 {
 	static const char *delimiters;
-	char *result;
+	char *ext = NULL;
+	char *iterate;
 	FILE *fd;
 	char line[1024];
-	char *mimetype;
 	char *token;
+	struct string_list_item *mime;
 
-	if (!filename)
-		return NULL;
+	*mimetype = NULL;
 
-	fd = fopen(filename, "r");
-	if (!fd)
-		return NULL;
+	if (!qrypath)
+		return;
 
-	delimiters = " \t\r\n";
-	result = NULL;
+	ext = strrchr(qrypath, '.');
 
-	/* loop over all lines in the file */
-	while (!result && fgets(line, sizeof(line), fd)) {
-		mimetype = strtok(line, delimiters);
+	if (ext && *(++ext)) {
+		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
+		if (mime) {
+			*mimetype = (char *)mime->util;
+		} else {
+			fd = fopen(ctx.cfg.mimetype_file, "r");
+			if (!fd)
+				return;
 
-		/* skip empty lines and comment lines */
-		if (!mimetype || (mimetype[0] == '#'))
-			continue;
+			delimiters = " \t\r\n";
 
-		/* loop over all extensions of mimetype */
-		while ((token = strtok(NULL, delimiters))) {
-			if (!strcasecmp(ext, token)) {
-				result = xstrdup(mimetype);
-				break;
+			/* loop over all lines in the file */
+			while (!*mimetype && fgets(line, sizeof(line), fd)) {
+				iterate = strtok(line, delimiters);
+
+				/* skip empty lines and comment lines */
+				if (!iterate || (iterate[0] == '#'))
+					continue;
+
+				/* loop over all extensions of mimetype */
+				while ((token = strtok(NULL, delimiters))) {
+					if (!strcasecmp(ext, token)) {
+						*mimetype = iterate;
+						break;
+					}
+				}
 			}
+			fclose(fd);
 		}
 	}
-	fclose(fd);
-
-	return result;
 }
 
diff --git a/ui-plain.c b/ui-plain.c
index d68518e..57c4afc 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -19,10 +19,8 @@ struct walk_tree_context {
 static int print_object(const unsigned char *sha1, const char *path)
 {
 	enum object_type type;
-	char *buf, *ext;
+	char *buf;
 	unsigned long size;
-	struct string_list_item *mime;
-	int freemime;
 
 	type = sha1_object_info(sha1, &size);
 	if (type == OBJ_BAD) {
@@ -36,21 +34,9 @@ static int print_object(const unsigned char *sha1, const char *path)
 		return 0;
 	}
 	ctx.page.mimetype = NULL;
-	ext = strrchr(path, '.');
-	freemime = 0;
-	if (ext && *(++ext)) {
-		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
-		if (mime) {
-			ctx.page.mimetype = (char *)mime->util;
-			ctx.page.charset = NULL;
-		} else {
-			ctx.page.mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
-			if (ctx.page.mimetype) {
-				freemime = 1;
-				ctx.page.charset = NULL;
-			}
-		}
-	}
+
+	get_mimetype_from_file(ctx.qry.path, &(ctx.page.mimetype));
+
 	if (!ctx.page.mimetype) {
 		if (buffer_is_binary(buf, size)) {
 			ctx.page.mimetype = "application/octet-stream";
@@ -64,9 +50,6 @@ static int print_object(const unsigned char *sha1, const char *path)
 	ctx.page.etag = sha1_to_hex(sha1);
 	cgit_print_http_headers();
 	html_raw(buf, size);
-	/* If we allocated this, then casting away const is safe. */
-	if (freemime)
-		free((char*) ctx.page.mimetype);
 	return 1;
 }
 
diff --git a/ui-summary.c b/ui-summary.c
index 99c9234..55bf84a 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -101,39 +101,19 @@ static char* append_readme_path(const char *filename, const char *ref, const cha
 
 void cgit_print_repo_readme(char *path)
 {
-	char *ext = NULL, *filename, *ref, *mimetype = NULL;
+	char *filename, *ref;
+	const char *mimetype;
 	int free_filename = 0;
-	int freemime = 0;
-	struct string_list_item *mime;
-
-	if (ctx.qry.path)
-		ext = strrchr(ctx.qry.path, '.');
-
-	if (ext && *(++ext)) {
-		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
-		if (mime) {
-			mimetype = (char *)mime->util;
-		} else {
-			mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
-			if (mimetype)
-				freemime = 1;
-		}
-	}
+
+	get_mimetype_from_file(ctx.qry.path, &mimetype);
 
 	if (mimetype && strncmp(mimetype, "image/", 6) == 0) {
 		ctx.page.mimetype = mimetype;
 		ctx.page.charset = NULL;
 		cgit_print_plain();
-
-		if (freemime)
-			free(mimetype);
-
 		return;
 	}
 
-	if (freemime)
-		free(mimetype);
-
 	cgit_print_layout_start();
 	if (ctx.repo->readme.nr == 0)
 		goto done;
-- 
2.5.0

[PATCH 2/3] about: send images plain

[Jason]

Excellent! This is a very wanted tweak indeed, and it's finally possible
thanks to the rework merged today that moves layout into each function.
Awesome.

I'm on a camping trip this weekend, so I'll get to merging this late on
Sunday or Monday morning.
On Aug 14, 2015 11:16 PM, "Christian Hesse" <list at eworm.de> wrote:

> From: Christian Hesse <mail at eworm.de>
>
> The about page used to display just fine, but images were broken: The
> binary image data was embedded in html code.
> Use cgit_print_plain() to send images in plain mode and make them
> available on about page.
>
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
>  ui-summary.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/ui-summary.c b/ui-summary.c
> index fb04dc3..99c9234 100644
> --- a/ui-summary.c
> +++ b/ui-summary.c
> @@ -9,9 +9,10 @@
>  #include "cgit.h"
>  #include "ui-summary.h"
>  #include "html.h"
> +#include "ui-blob.h"
>  #include "ui-log.h"
> +#include "ui-plain.h"
>  #include "ui-refs.h"
> -#include "ui-blob.h"
>  #include "ui-shared.h"
>
>  static int urls;
> @@ -100,8 +101,38 @@ static char* append_readme_path(const char *filename,
> const char *ref, const cha
>
>  void cgit_print_repo_readme(char *path)
>  {
> -       char *filename, *ref;
> +       char *ext = NULL, *filename, *ref, *mimetype = NULL;
>         int free_filename = 0;
> +       int freemime = 0;
> +       struct string_list_item *mime;
> +
> +       if (ctx.qry.path)
> +               ext = strrchr(ctx.qry.path, '.');
> +
> +       if (ext && *(++ext)) {
> +               mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> +               if (mime) {
> +                       mimetype = (char *)mime->util;
> +               } else {
> +                       mimetype =
> get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
> +                       if (mimetype)
> +                               freemime = 1;
> +               }
> +       }
> +
> +       if (mimetype && strncmp(mimetype, "image/", 6) == 0) {
> +               ctx.page.mimetype = mimetype;
> +               ctx.page.charset = NULL;
> +               cgit_print_plain();
> +
> +               if (freemime)
> +                       free(mimetype);
> +
> +               return;
> +       }
> +
> +       if (freemime)
> +               free(mimetype);
>
>         cgit_print_layout_start();
>         if (ctx.repo->readme.nr == 0)
> --
> 2.5.0
>
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20150815/0d2b36e7/attachment-0001.html>

[PATCH 3/3] move shared code to get_mimetype_from_file()

[john]

On Fri, Aug 14, 2015 at 11:16:37PM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
>  cgit.h       |  2 +-
>  shared.c     | 57 +++++++++++++++++++++++++++++++++------------------------
>  ui-plain.c   | 25 ++++---------------------
>  ui-summary.c | 28 ++++------------------------
>  4 files changed, 42 insertions(+), 70 deletions(-)
> 
> diff --git a/cgit.h b/cgit.h
> index 0f1e186..dfa6d0c 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -391,6 +391,6 @@ extern int readfile(const char *path, char **buf, size_t *size);
>  
>  extern char *expand_macros(const char *txt);
>  
> -extern char *get_mimetype_from_file(const char *filename, const char *ext);
> +extern void get_mimetype_from_file(const char *qrypath, const char **mimetype);
>  
>  #endif /* CGIT_H */
> diff --git a/shared.c b/shared.c
> index 5a000a6..1f049e7 100644
> --- a/shared.c
> +++ b/shared.c
> @@ -561,43 +561,52 @@ char *expand_macros(const char *txt)
>  	return result;
>  }
>  
> -char *get_mimetype_from_file(const char *filename, const char *ext)
> +void get_mimetype_from_file(const char *qrypath, const char **mimetype)
>  {
>  	static const char *delimiters;
> -	char *result;
> +	char *ext = NULL;
> +	char *iterate;
>  	FILE *fd;
>  	char line[1024];
> -	char *mimetype;
>  	char *token;
> +	struct string_list_item *mime;
>  
> -	if (!filename)
> -		return NULL;
> +	*mimetype = NULL;
>  
> -	fd = fopen(filename, "r");
> -	if (!fd)
> -		return NULL;
> +	if (!qrypath)
> +		return;
>  
> -	delimiters = " \t\r\n";
> -	result = NULL;
> +	ext = strrchr(qrypath, '.');
>  
> -	/* loop over all lines in the file */
> -	while (!result && fgets(line, sizeof(line), fd)) {
> -		mimetype = strtok(line, delimiters);
> +	if (ext && *(++ext)) {
> +		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> +		if (mime) {
> +			*mimetype = (char *)mime->util;
> +		} else {
> +			fd = fopen(ctx.cfg.mimetype_file, "r");
> +			if (!fd)
> +				return;
>  
> -		/* skip empty lines and comment lines */
> -		if (!mimetype || (mimetype[0] == '#'))
> -			continue;
> +			delimiters = " \t\r\n";
>  
> -		/* loop over all extensions of mimetype */
> -		while ((token = strtok(NULL, delimiters))) {
> -			if (!strcasecmp(ext, token)) {
> -				result = xstrdup(mimetype);
> -				break;
> +			/* loop over all lines in the file */
> +			while (!*mimetype && fgets(line, sizeof(line), fd)) {
> +				iterate = strtok(line, delimiters);
> +
> +				/* skip empty lines and comment lines */
> +				if (!iterate || (iterate[0] == '#'))
> +					continue;
> +
> +				/* loop over all extensions of mimetype */
> +				while ((token = strtok(NULL, delimiters))) {
> +					if (!strcasecmp(ext, token)) {
> +						*mimetype = iterate;

Doesn't this result in us reading stale memory in the caller?
"iterate" derives from "line" which is on the stack.

> +						break;
> +					}
> +				}
>  			}
> +			fclose(fd);
>  		}
>  	}
> -	fclose(fd);
> -
> -	return result;
>  }
>  
> diff --git a/ui-plain.c b/ui-plain.c
> index d68518e..57c4afc 100644
> --- a/ui-plain.c
> +++ b/ui-plain.c
> @@ -19,10 +19,8 @@ struct walk_tree_context {
>  static int print_object(const unsigned char *sha1, const char *path)
>  {
>  	enum object_type type;
> -	char *buf, *ext;
> +	char *buf;
>  	unsigned long size;
> -	struct string_list_item *mime;
> -	int freemime;
>  
>  	type = sha1_object_info(sha1, &size);
>  	if (type == OBJ_BAD) {
> @@ -36,21 +34,9 @@ static int print_object(const unsigned char *sha1, const char *path)
>  		return 0;
>  	}
>  	ctx.page.mimetype = NULL;
> -	ext = strrchr(path, '.');
> -	freemime = 0;
> -	if (ext && *(++ext)) {
> -		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> -		if (mime) {
> -			ctx.page.mimetype = (char *)mime->util;
> -			ctx.page.charset = NULL;
> -		} else {
> -			ctx.page.mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
> -			if (ctx.page.mimetype) {
> -				freemime = 1;
> -				ctx.page.charset = NULL;
> -			}
> -		}
> -	}
> +
> +	get_mimetype_from_file(ctx.qry.path, &(ctx.page.mimetype));
> +
>  	if (!ctx.page.mimetype) {
>  		if (buffer_is_binary(buf, size)) {
>  			ctx.page.mimetype = "application/octet-stream";
> @@ -64,9 +50,6 @@ static int print_object(const unsigned char *sha1, const char *path)
>  	ctx.page.etag = sha1_to_hex(sha1);
>  	cgit_print_http_headers();
>  	html_raw(buf, size);
> -	/* If we allocated this, then casting away const is safe. */
> -	if (freemime)
> -		free((char*) ctx.page.mimetype);
>  	return 1;
>  }
>  
> diff --git a/ui-summary.c b/ui-summary.c
> index 99c9234..55bf84a 100644
> --- a/ui-summary.c
> +++ b/ui-summary.c
> @@ -101,39 +101,19 @@ static char* append_readme_path(const char *filename, const char *ref, const cha
>  
>  void cgit_print_repo_readme(char *path)
>  {
> -	char *ext = NULL, *filename, *ref, *mimetype = NULL;
> +	char *filename, *ref;
> +	const char *mimetype;
>  	int free_filename = 0;
> -	int freemime = 0;
> -	struct string_list_item *mime;
> -
> -	if (ctx.qry.path)
> -		ext = strrchr(ctx.qry.path, '.');
> -
> -	if (ext && *(++ext)) {
> -		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> -		if (mime) {
> -			mimetype = (char *)mime->util;
> -		} else {
> -			mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
> -			if (mimetype)
> -				freemime = 1;
> -		}
> -	}
> +
> +	get_mimetype_from_file(ctx.qry.path, &mimetype);
>  
>  	if (mimetype && strncmp(mimetype, "image/", 6) == 0) {
>  		ctx.page.mimetype = mimetype;
>  		ctx.page.charset = NULL;
>  		cgit_print_plain();
> -
> -		if (freemime)
> -			free(mimetype);
> -
>  		return;
>  	}
>  
> -	if (freemime)
> -		free(mimetype);
> -
>  	cgit_print_layout_start();
>  	if (ctx.repo->readme.nr == 0)
>  		goto done;
> -- 
> 2.5.0
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit

[PATCH 2/3] about: send images plain

[john]

On Fri, Aug 14, 2015 at 11:16:36PM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> The about page used to display just fine, but images were broken: The
> binary image data was embedded in html code.
> Use cgit_print_plain() to send images in plain mode and make them
> available on about page.
> 
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
>  ui-summary.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/ui-summary.c b/ui-summary.c
> index fb04dc3..99c9234 100644
> --- a/ui-summary.c
> +++ b/ui-summary.c
> @@ -9,9 +9,10 @@
>  #include "cgit.h"
>  #include "ui-summary.h"
>  #include "html.h"
> +#include "ui-blob.h"
>  #include "ui-log.h"
> +#include "ui-plain.h"
>  #include "ui-refs.h"
> -#include "ui-blob.h"
>  #include "ui-shared.h"
>  
>  static int urls;
> @@ -100,8 +101,38 @@ static char* append_readme_path(const char *filename, const char *ref, const cha
>  
>  void cgit_print_repo_readme(char *path)
>  {
> -	char *filename, *ref;
> +	char *ext = NULL, *filename, *ref, *mimetype = NULL;
>  	int free_filename = 0;
> +	int freemime = 0;
> +	struct string_list_item *mime;
> +
> +	if (ctx.qry.path)
> +		ext = strrchr(ctx.qry.path, '.');
> +
> +	if (ext && *(++ext)) {
> +		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> +		if (mime) {
> +			mimetype = (char *)mime->util;
> +		} else {
> +			mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
> +			if (mimetype)
> +				freemime = 1;
> +		}
> +	}

This would be simpler if the refactoring in patch 3 was done as a
preparatory step rather than a clean up afterwards.

> +	if (mimetype && strncmp(mimetype, "image/", 6) == 0) {
> +		ctx.page.mimetype = mimetype;
> +		ctx.page.charset = NULL;
> +		cgit_print_plain();
> +
> +		if (freemime)
> +			free(mimetype);
> +
> +		return;
> +	}
> +
> +	if (freemime)
> +		free(mimetype);
>  
>  	cgit_print_layout_start();
>  	if (ctx.repo->readme.nr == 0)
> -- 
> 2.5.0
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit

[PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath()

[list]

From: Christian Hesse <mail at eworm.de>

* handle mimetype within a single function
+ return allocated memory on success

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

diff --git a/cgit.h b/cgit.h
index 0f1e186..d34d353 100644
--- a/cgit.h
+++ b/cgit.h
@@ -391,6 +391,6 @@ extern int readfile(const char *path, char **buf, size_t *size);
 
 extern char *expand_macros(const char *txt);
 
-extern char *get_mimetype_from_file(const char *filename, const char *ext);
+extern char *get_mimetype_for_qrypath(const char *qrypath);
 
 #endif /* CGIT_H */
diff --git a/shared.c b/shared.c
index 5a000a6..0f1f8de 100644
--- a/shared.c
+++ b/shared.c
@@ -561,43 +561,52 @@ char *expand_macros(const char *txt)
 	return result;
 }
 
-char *get_mimetype_from_file(const char *filename, const char *ext)
+char *get_mimetype_for_qrypath(const char *qrypath)
 {
 	static const char *delimiters;
-	char *result;
-	FILE *fd;
+	char *ext = NULL, *iterate, *mimetype = NULL, *token;
 	char line[1024];
-	char *mimetype;
-	char *token;
-
-	if (!filename)
-		return NULL;
+	FILE *fd;
+	struct string_list_item *mime;
 
-	fd = fopen(filename, "r");
-	if (!fd)
+	if (qrypath == NULL)
 		return NULL;
 
-	delimiters = " \t\r\n";
-	result = NULL;
-
-	/* loop over all lines in the file */
-	while (!result && fgets(line, sizeof(line), fd)) {
-		mimetype = strtok(line, delimiters);
-
-		/* skip empty lines and comment lines */
-		if (!mimetype || (mimetype[0] == '#'))
-			continue;
-
-		/* loop over all extensions of mimetype */
-		while ((token = strtok(NULL, delimiters))) {
-			if (!strcasecmp(ext, token)) {
-				result = xstrdup(mimetype);
-				break;
+	ext = strrchr(qrypath, '.');
+
+	if (ext && *(++ext)) {
+		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
+		if (mime) {
+			/* We could just pass the pointer here, but would have to care
+			 * whether or not to free the memory. Instead just dup. */
+			mimetype = xstrdup(mime->util);
+		} else {
+			fd = fopen(ctx.cfg.mimetype_file, "r");
+			if (fd == NULL)
+				return NULL;
+
+			delimiters = " \t\r\n";
+
+			/* loop over all lines in the file */
+			while (mimetype == NULL && fgets(line, sizeof(line), fd)) {
+				iterate = strtok(line, delimiters);
+
+				/* skip empty lines and comment lines */
+				if (iterate == NULL || (iterate[0] == '#'))
+					continue;
+
+				/* loop over all extensions of mimetype */
+				while ((token = strtok(NULL, delimiters))) {
+					if (strcasecmp(ext, token) == 0) {
+						mimetype = xstrdup(iterate);
+						break;
+					}
+				}
 			}
+			fclose(fd);
 		}
 	}
-	fclose(fd);
 
-	return result;
+	return mimetype;
 }
 
diff --git a/ui-plain.c b/ui-plain.c
index d68518e..ce59415 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -19,10 +19,8 @@ struct walk_tree_context {
 static int print_object(const unsigned char *sha1, const char *path)
 {
 	enum object_type type;
-	char *buf, *ext;
+	char *buf, *mimetype;
 	unsigned long size;
-	struct string_list_item *mime;
-	int freemime;
 
 	type = sha1_object_info(sha1, &size);
 	if (type == OBJ_BAD) {
@@ -35,22 +33,10 @@ static int print_object(const unsigned char *sha1, const char *path)
 		cgit_print_error_page(404, "Not found", "Not found");
 		return 0;
 	}
-	ctx.page.mimetype = NULL;
-	ext = strrchr(path, '.');
-	freemime = 0;
-	if (ext && *(++ext)) {
-		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
-		if (mime) {
-			ctx.page.mimetype = (char *)mime->util;
-			ctx.page.charset = NULL;
-		} else {
-			ctx.page.mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
-			if (ctx.page.mimetype) {
-				freemime = 1;
-				ctx.page.charset = NULL;
-			}
-		}
-	}
+
+	mimetype = get_mimetype_for_qrypath(ctx.qry.path);
+	ctx.page.mimetype = mimetype;
+
 	if (!ctx.page.mimetype) {
 		if (buffer_is_binary(buf, size)) {
 			ctx.page.mimetype = "application/octet-stream";
@@ -64,9 +50,8 @@ static int print_object(const unsigned char *sha1, const char *path)
 	ctx.page.etag = sha1_to_hex(sha1);
 	cgit_print_http_headers();
 	html_raw(buf, size);
-	/* If we allocated this, then casting away const is safe. */
-	if (freemime)
-		free((char*) ctx.page.mimetype);
+	if (mimetype)
+		free(mimetype);
 	return 1;
 }
 
-- 
2.5.0

[PATCH 3/3] ui-summary: send images plain for about page

[list]

From: Christian Hesse <mail at eworm.de>

The about page used to display just fine, but images were broken: The
binary image data was embedded in html code.
Use cgit_print_plain() to send images in plain mode and make them
available on about page.

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

diff --git a/ui-summary.c b/ui-summary.c
index fb04dc3..09ec379 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -9,9 +9,10 @@
 #include "cgit.h"
 #include "ui-summary.h"
 #include "html.h"
+#include "ui-blob.h"
 #include "ui-log.h"
+#include "ui-plain.h"
 #include "ui-refs.h"
-#include "ui-blob.h"
 #include "ui-shared.h"
 
 static int urls;
@@ -100,9 +101,22 @@ static char* append_readme_path(const char *filename, const char *ref, const cha
 
 void cgit_print_repo_readme(char *path)
 {
-	char *filename, *ref;
+	char *filename, *ref, *mimetype;
 	int free_filename = 0;
 
+	mimetype = get_mimetype_for_qrypath(ctx.qry.path);
+
+	if (mimetype && strncmp(mimetype, "image/", 6) == 0) {
+		ctx.page.mimetype = mimetype;
+		ctx.page.charset = NULL;
+		cgit_print_plain();
+		free(mimetype);
+		return;
+	}
+
+	if (mimetype)
+		free(mimetype);
+
 	cgit_print_layout_start();
 	if (ctx.repo->readme.nr == 0)
 		goto done;
-- 
2.5.0

[PATCH 2/3] about: send images plain

[list]

"Jason A. Donenfeld" <Jason at zx2c4.com> on Sat, 2015/08/15 01:07:
> Excellent! This is a very wanted tweak indeed, and it's finally possible
> thanks to the rework merged today that moves layout into each function.
> Awesome.

Yes, really glad we have the layout changes. ;)

Just updated my patches after John's review. Wait for him to agree. :-p

> I'm on a camping trip this weekend, so I'll get to merging this late on
> Sunday or Monday morning.

Have a nice weekend then!
-- 
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/20150815/99662ed9/attachment.asc>

[PATCH 3/3] move shared code to get_mimetype_from_file()

[list]

John Keeping <john at keeping.me.uk> on Sat, 2015/08/15 12:29:
> > +			/* loop over all lines in the file */
> > +			while (!*mimetype && fgets(line, sizeof(line),
> > fd)) {
> > +				iterate = strtok(line, delimiters);
> > +
> > +				/* skip empty lines and comment lines */
> > +				if (!iterate || (iterate[0] == '#'))
> > +					continue;
> > +
> > +				/* loop over all extensions of mimetype
> > */
> > +				while ((token = strtok(NULL,
> > delimiters))) {
> > +					if (!strcasecmp(ext, token)) {
> > +						*mimetype = iterate;  
> 
> Doesn't this result in us reading stale memory in the caller?
> "iterate" derives from "line" which is on the stack.

This used to work, probably just as long as we do not call strtok() again. I
addressed that in my updated patches.
-- 
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/20150815/d43619c3/attachment-0001.asc>

[PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath()

[john]

On Sat, Aug 15, 2015 at 10:08:22PM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> * handle mimetype within a single function
> + return allocated memory on success
> 
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
>  cgit.h     |  2 +-
>  shared.c   | 65 +++++++++++++++++++++++++++++++++++---------------------------
>  ui-plain.c | 29 +++++++---------------------
>  3 files changed, 45 insertions(+), 51 deletions(-)
> 
> diff --git a/cgit.h b/cgit.h
> index 0f1e186..d34d353 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -391,6 +391,6 @@ extern int readfile(const char *path, char **buf, size_t *size);
>  
>  extern char *expand_macros(const char *txt);
>  
> -extern char *get_mimetype_from_file(const char *filename, const char *ext);
> +extern char *get_mimetype_for_qrypath(const char *qrypath);
>  
>  #endif /* CGIT_H */
> diff --git a/shared.c b/shared.c
> index 5a000a6..0f1f8de 100644
> --- a/shared.c
> +++ b/shared.c
> @@ -561,43 +561,52 @@ char *expand_macros(const char *txt)
>  	return result;
>  }
>  
> -char *get_mimetype_from_file(const char *filename, const char *ext)
> +char *get_mimetype_for_qrypath(const char *qrypath)

If this is called get_mimetype_for_qrypath(), shouldn't it just use
"ctx.qry.path" without taking an argument?  Personally, I'd prefer
get_mimetype_for_filename() taking a "path" argument (also see below
about "path" vs. "ctx.qry.path").

>  {
>  	static const char *delimiters;
> -	char *result;
> -	FILE *fd;
> +	char *ext = NULL, *iterate, *mimetype = NULL, *token;
>  	char line[1024];
> -	char *mimetype;
> -	char *token;
> -
> -	if (!filename)
> -		return NULL;
> +	FILE *fd;
> +	struct string_list_item *mime;
>  
> -	fd = fopen(filename, "r");
> -	if (!fd)
> +	if (qrypath == NULL)
>  		return NULL;
>  
> -	delimiters = " \t\r\n";
> -	result = NULL;
> -
> -	/* loop over all lines in the file */
> -	while (!result && fgets(line, sizeof(line), fd)) {
> -		mimetype = strtok(line, delimiters);
> -
> -		/* skip empty lines and comment lines */
> -		if (!mimetype || (mimetype[0] == '#'))
> -			continue;
> -
> -		/* loop over all extensions of mimetype */
> -		while ((token = strtok(NULL, delimiters))) {
> -			if (!strcasecmp(ext, token)) {
> -				result = xstrdup(mimetype);
> -				break;
> +	ext = strrchr(qrypath, '.');
> +
> +	if (ext && *(++ext)) {
> +		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> +		if (mime) {
> +			/* We could just pass the pointer here, but would have to care
> +			 * whether or not to free the memory. Instead just dup. */
> +			mimetype = xstrdup(mime->util);
> +		} else {

Should this be:

		} else if (ctx.cfg.mimetype_file) {

?  Above there's a deleted check on "filename" in the original
get_mimetype_from_file() function.

> +			fd = fopen(ctx.cfg.mimetype_file, "r");
> +			if (fd == NULL)
> +				return NULL;
> +
> +			delimiters = " \t\r\n";
> +
> +			/* loop over all lines in the file */
> +			while (mimetype == NULL && fgets(line, sizeof(line), fd)) {
> +				iterate = strtok(line, delimiters);
> +
> +				/* skip empty lines and comment lines */
> +				if (iterate == NULL || (iterate[0] == '#'))
> +					continue;
> +
> +				/* loop over all extensions of mimetype */
> +				while ((token = strtok(NULL, delimiters))) {
> +					if (strcasecmp(ext, token) == 0) {
> +						mimetype = xstrdup(iterate);
> +						break;
> +					}
> +				}
>  			}
> +			fclose(fd);
>  		}
>  	}
> -	fclose(fd);
>  
> -	return result;
> +	return mimetype;
>  }
>  
> diff --git a/ui-plain.c b/ui-plain.c
> index d68518e..ce59415 100644
> --- a/ui-plain.c
> +++ b/ui-plain.c
> @@ -19,10 +19,8 @@ struct walk_tree_context {
>  static int print_object(const unsigned char *sha1, const char *path)
>  {
>  	enum object_type type;
> -	char *buf, *ext;
> +	char *buf, *mimetype;
>  	unsigned long size;
> -	struct string_list_item *mime;
> -	int freemime;
>  
>  	type = sha1_object_info(sha1, &size);
>  	if (type == OBJ_BAD) {
> @@ -35,22 +33,10 @@ static int print_object(const unsigned char *sha1, const char *path)
>  		cgit_print_error_page(404, "Not found", "Not found");
>  		return 0;
>  	}
> -	ctx.page.mimetype = NULL;
> -	ext = strrchr(path, '.');
> -	freemime = 0;
> -	if (ext && *(++ext)) {
> -		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> -		if (mime) {
> -			ctx.page.mimetype = (char *)mime->util;
> -			ctx.page.charset = NULL;
> -		} else {
> -			ctx.page.mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
> -			if (ctx.page.mimetype) {
> -				freemime = 1;
> -				ctx.page.charset = NULL;
> -			}
> -		}
> -	}
> +
> +	mimetype = get_mimetype_for_qrypath(ctx.qry.path);

This was "path" before, not "ctx.qry.path".  It looks like they will
always be the same, but we're guaranteed that "path" is non-null.

> +	ctx.page.mimetype = mimetype;
> +
>  	if (!ctx.page.mimetype) {
>  		if (buffer_is_binary(buf, size)) {
>  			ctx.page.mimetype = "application/octet-stream";
> @@ -64,9 +50,8 @@ static int print_object(const unsigned char *sha1, const char *path)
>  	ctx.page.etag = sha1_to_hex(sha1);
>  	cgit_print_http_headers();
>  	html_raw(buf, size);
> -	/* If we allocated this, then casting away const is safe. */
> -	if (freemime)
> -		free((char*) ctx.page.mimetype);
> +	if (mimetype)

There's no need to check "mimetype" before calling free(3), since
free(3) is defined to be a no-op if it's passed a null pointer.

> +		free(mimetype);
>  	return 1;
>  }
>  
> -- 
> 2.5.0

[PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath()

[list]

John Keeping <john at keeping.me.uk> on Sun, 2015/08/16 13:19:
> On Sat, Aug 15, 2015 at 10:08:22PM +0200, Christian Hesse wrote:
> > From: Christian Hesse <mail at eworm.de>
> > 
> > * handle mimetype within a single function
> > + return allocated memory on success
> > 
> > Signed-off-by: Christian Hesse <mail at eworm.de>
> > ---
> >  cgit.h     |  2 +-
> >  shared.c   | 65
> > +++++++++++++++++++++++++++++++++++--------------------------- ui-plain.c
> > | 29 +++++++--------------------- 3 files changed, 45 insertions(+), 51
> > deletions(-)
> > 
> > diff --git a/cgit.h b/cgit.h
> > index 0f1e186..d34d353 100644
> > --- a/cgit.h
> > +++ b/cgit.h
> > @@ -391,6 +391,6 @@ extern int readfile(const char *path, char **buf,
> > size_t *size); 
> >  extern char *expand_macros(const char *txt);
> >  
> > -extern char *get_mimetype_from_file(const char *filename, const char
> > *ext); +extern char *get_mimetype_for_qrypath(const char *qrypath);
> >  
> >  #endif /* CGIT_H */
> > diff --git a/shared.c b/shared.c
> > index 5a000a6..0f1f8de 100644
> > --- a/shared.c
> > +++ b/shared.c
> > @@ -561,43 +561,52 @@ char *expand_macros(const char *txt)
> >  	return result;
> >  }
> >  
> > -char *get_mimetype_from_file(const char *filename, const char *ext)
> > +char *get_mimetype_for_qrypath(const char *qrypath)
> 
> If this is called get_mimetype_for_qrypath(), shouldn't it just use
> "ctx.qry.path" without taking an argument?  Personally, I'd prefer
> get_mimetype_for_filename() taking a "path" argument (also see below
> about "path" vs. "ctx.qry.path").

At some point I was confused about ctx.qry.path and ctx.cfg.mimetypes. That
was the result I think... But you are right or course.

> >  {
> >  	static const char *delimiters;
> > -	char *result;
> > -	FILE *fd;
> > +	char *ext = NULL, *iterate, *mimetype = NULL, *token;
> >  	char line[1024];
> > -	char *mimetype;
> > -	char *token;
> > -
> > -	if (!filename)
> > -		return NULL;
> > +	FILE *fd;
> > +	struct string_list_item *mime;
> >  
> > -	fd = fopen(filename, "r");
> > -	if (!fd)
> > +	if (qrypath == NULL)
> >  		return NULL;
> >  
> > -	delimiters = " \t\r\n";
> > -	result = NULL;
> > -
> > -	/* loop over all lines in the file */
> > -	while (!result && fgets(line, sizeof(line), fd)) {
> > -		mimetype = strtok(line, delimiters);
> > -
> > -		/* skip empty lines and comment lines */
> > -		if (!mimetype || (mimetype[0] == '#'))
> > -			continue;
> > -
> > -		/* loop over all extensions of mimetype */
> > -		while ((token = strtok(NULL, delimiters))) {
> > -			if (!strcasecmp(ext, token)) {
> > -				result = xstrdup(mimetype);
> > -				break;
> > +	ext = strrchr(qrypath, '.');
> > +
> > +	if (ext && *(++ext)) {
> > +		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> > +		if (mime) {
> > +			/* We could just pass the pointer here, but
> > would have to care
> > +			 * whether or not to free the memory. Instead
> > just dup. */
> > +			mimetype = xstrdup(mime->util);
> > +		} else {
> 
> Should this be:
> 
> 		} else if (ctx.cfg.mimetype_file) {
> 
> ?  Above there's a deleted check on "filename" in the original
> get_mimetype_from_file() function.

Sure. Kind of related to the above.

> > +			fd = fopen(ctx.cfg.mimetype_file, "r");
> > +			if (fd == NULL)
> > +				return NULL;
> > +
> > +			delimiters = " \t\r\n";
> > +
> > +			/* loop over all lines in the file */
> > +			while (mimetype == NULL && fgets(line,
> > sizeof(line), fd)) {
> > +				iterate = strtok(line, delimiters);
> > +
> > +				/* skip empty lines and comment lines */
> > +				if (iterate == NULL || (iterate[0] ==
> > '#'))
> > +					continue;
> > +
> > +				/* loop over all extensions of mimetype
> > */
> > +				while ((token = strtok(NULL,
> > delimiters))) {
> > +					if (strcasecmp(ext, token) == 0)
> > {
> > +						mimetype =
> > xstrdup(iterate);
> > +						break;
> > +					}
> > +				}
> >  			}
> > +			fclose(fd);
> >  		}
> >  	}
> > -	fclose(fd);
> >  
> > -	return result;
> > +	return mimetype;
> >  }
> >  
> > diff --git a/ui-plain.c b/ui-plain.c
> > index d68518e..ce59415 100644
> > --- a/ui-plain.c
> > +++ b/ui-plain.c
> > @@ -19,10 +19,8 @@ struct walk_tree_context {
> >  static int print_object(const unsigned char *sha1, const char *path)
> >  {
> >  	enum object_type type;
> > -	char *buf, *ext;
> > +	char *buf, *mimetype;
> >  	unsigned long size;
> > -	struct string_list_item *mime;
> > -	int freemime;
> >  
> >  	type = sha1_object_info(sha1, &size);
> >  	if (type == OBJ_BAD) {
> > @@ -35,22 +33,10 @@ static int print_object(const unsigned char *sha1,
> > const char *path) cgit_print_error_page(404, "Not found", "Not found");
> >  		return 0;
> >  	}
> > -	ctx.page.mimetype = NULL;
> > -	ext = strrchr(path, '.');
> > -	freemime = 0;
> > -	if (ext && *(++ext)) {
> > -		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
> > -		if (mime) {
> > -			ctx.page.mimetype = (char *)mime->util;
> > -			ctx.page.charset = NULL;
> > -		} else {
> > -			ctx.page.mimetype =
> > get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
> > -			if (ctx.page.mimetype) {
> > -				freemime = 1;
> > -				ctx.page.charset = NULL;
> > -			}
> > -		}
> > -	}
> > +
> > +	mimetype = get_mimetype_for_qrypath(ctx.qry.path);
> 
> This was "path" before, not "ctx.qry.path".  It looks like they will
> always be the same, but we're guaranteed that "path" is non-null.

And again... It's correct to use "path".

> > +	ctx.page.mimetype = mimetype;
> > +
> >  	if (!ctx.page.mimetype) {
> >  		if (buffer_is_binary(buf, size)) {
> >  			ctx.page.mimetype = "application/octet-stream";
> > @@ -64,9 +50,8 @@ static int print_object(const unsigned char *sha1,
> > const char *path) ctx.page.etag = sha1_to_hex(sha1);
> >  	cgit_print_http_headers();
> >  	html_raw(buf, size);
> > -	/* If we allocated this, then casting away const is safe. */
> > -	if (freemime)
> > -		free((char*) ctx.page.mimetype);
> > +	if (mimetype)
> 
> There's no need to check "mimetype" before calling free(3), since
> free(3) is defined to be a no-op if it's passed a null pointer.

Ah, did not know that. Nice!

Another good reason to initialize pointer with NULL. :D

> > +		free(mimetype);
> >  	return 1;
> >  }
> >  
> > -- 
> > 2.5.0



-- 
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/20150816/53507716/attachment.asc>

[PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_for_filename()

[list]

From: Christian Hesse <mail at eworm.de>

* handle mimetype within a single function
* return allocated memory on success

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

diff --git a/cgit.h b/cgit.h
index 0f1e186..b7eccdd 100644
--- a/cgit.h
+++ b/cgit.h
@@ -391,6 +391,6 @@ extern int readfile(const char *path, char **buf, size_t *size);
 
 extern char *expand_macros(const char *txt);
 
-extern char *get_mimetype_from_file(const char *filename, const char *ext);
+extern char *get_mimetype_for_filename(const char *filename);
 
 #endif /* CGIT_H */
diff --git a/shared.c b/shared.c
index 5a000a6..f035482 100644
--- a/shared.c
+++ b/shared.c
@@ -561,43 +561,52 @@ char *expand_macros(const char *txt)
 	return result;
 }
 
-char *get_mimetype_from_file(const char *filename, const char *ext)
+char *get_mimetype_for_filename(const char *filename)
 {
 	static const char *delimiters;
-	char *result;
-	FILE *fd;
+	char *ext = NULL, *iterate, *mimetype = NULL, *token;
 	char line[1024];
-	char *mimetype;
-	char *token;
-
-	if (!filename)
-		return NULL;
+	FILE *fd;
+	struct string_list_item *mime;
 
-	fd = fopen(filename, "r");
-	if (!fd)
+	if (filename == NULL)
 		return NULL;
 
-	delimiters = " \t\r\n";
-	result = NULL;
-
-	/* loop over all lines in the file */
-	while (!result && fgets(line, sizeof(line), fd)) {
-		mimetype = strtok(line, delimiters);
-
-		/* skip empty lines and comment lines */
-		if (!mimetype || (mimetype[0] == '#'))
-			continue;
-
-		/* loop over all extensions of mimetype */
-		while ((token = strtok(NULL, delimiters))) {
-			if (!strcasecmp(ext, token)) {
-				result = xstrdup(mimetype);
-				break;
+	ext = strrchr(filename, '.');
+
+	if (ext && *(++ext)) {
+		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
+		if (mime) {
+			/* We could just pass the pointer here, but would have to care
+			 * whether or not to free the memory. Instead just dup. */
+			mimetype = xstrdup(mime->util);
+		} else if (ctx.cfg.mimetype_file != NULL) {
+			fd = fopen(ctx.cfg.mimetype_file, "r");
+			if (fd == NULL)
+				return NULL;
+
+			delimiters = " \t\r\n";
+
+			/* loop over all lines in the file */
+			while (mimetype == NULL && fgets(line, sizeof(line), fd)) {
+				iterate = strtok(line, delimiters);
+
+				/* skip empty lines and comment lines */
+				if (iterate == NULL || (iterate[0] == '#'))
+					continue;
+
+				/* loop over all extensions of mimetype */
+				while ((token = strtok(NULL, delimiters))) {
+					if (strcasecmp(ext, token) == 0) {
+						mimetype = xstrdup(iterate);
+						break;
+					}
+				}
 			}
+			fclose(fd);
 		}
 	}
-	fclose(fd);
 
-	return result;
+	return mimetype;
 }
 
diff --git a/ui-plain.c b/ui-plain.c
index d68518e..0dd1a8b 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -19,10 +19,8 @@ struct walk_tree_context {
 static int print_object(const unsigned char *sha1, const char *path)
 {
 	enum object_type type;
-	char *buf, *ext;
+	char *buf, *mimetype;
 	unsigned long size;
-	struct string_list_item *mime;
-	int freemime;
 
 	type = sha1_object_info(sha1, &size);
 	if (type == OBJ_BAD) {
@@ -35,22 +33,10 @@ static int print_object(const unsigned char *sha1, const char *path)
 		cgit_print_error_page(404, "Not found", "Not found");
 		return 0;
 	}
-	ctx.page.mimetype = NULL;
-	ext = strrchr(path, '.');
-	freemime = 0;
-	if (ext && *(++ext)) {
-		mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
-		if (mime) {
-			ctx.page.mimetype = (char *)mime->util;
-			ctx.page.charset = NULL;
-		} else {
-			ctx.page.mimetype = get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
-			if (ctx.page.mimetype) {
-				freemime = 1;
-				ctx.page.charset = NULL;
-			}
-		}
-	}
+
+	mimetype = get_mimetype_for_filename(path);
+	ctx.page.mimetype = mimetype;
+
 	if (!ctx.page.mimetype) {
 		if (buffer_is_binary(buf, size)) {
 			ctx.page.mimetype = "application/octet-stream";
@@ -64,9 +50,7 @@ static int print_object(const unsigned char *sha1, const char *path)
 	ctx.page.etag = sha1_to_hex(sha1);
 	cgit_print_http_headers();
 	html_raw(buf, size);
-	/* If we allocated this, then casting away const is safe. */
-	if (freemime)
-		free((char*) ctx.page.mimetype);
+	free(mimetype);
 	return 1;
 }
 
-- 
2.5.0

[PATCH 3/3] ui-summary: send images plain for about page

[list]

From: Christian Hesse <mail at eworm.de>

The about page used to display just fine, but images were broken: The
binary image data was embedded in html code.
Use cgit_print_plain() to send images in plain mode and make them
available on about page.

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

diff --git a/ui-summary.c b/ui-summary.c
index fb04dc3..10b2bfc 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -9,9 +9,10 @@
 #include "cgit.h"
 #include "ui-summary.h"
 #include "html.h"
+#include "ui-blob.h"
 #include "ui-log.h"
+#include "ui-plain.h"
 #include "ui-refs.h"
-#include "ui-blob.h"
 #include "ui-shared.h"
 
 static int urls;
@@ -100,9 +101,21 @@ static char* append_readme_path(const char *filename, const char *ref, const cha
 
 void cgit_print_repo_readme(char *path)
 {
-	char *filename, *ref;
+	char *filename, *ref, *mimetype;
 	int free_filename = 0;
 
+	mimetype = get_mimetype_for_filename(path);
+
+	if (mimetype && strncmp(mimetype, "image/", 6) == 0) {
+		ctx.page.mimetype = mimetype;
+		ctx.page.charset = NULL;
+		cgit_print_plain();
+		free(mimetype);
+		return;
+	}
+
+	free(mimetype);
+
 	cgit_print_layout_start();
 	if (ctx.repo->readme.nr == 0)
 		goto done;
-- 
2.5.0