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

[John Keeping]

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