* [PATCH 1/2] move get_mimetype_from_file() to shared
@ 2015-08-14 14:50 list
2015-08-14 14:50 ` [PATCH 2/2] about: send images plain list
0 siblings, 1 reply; 16+ messages in thread
From: list @ 2015-08-14 14:50 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/2] about: send images plain
2015-08-14 14:50 [PATCH 1/2] move get_mimetype_from_file() to shared list
@ 2015-08-14 14:50 ` list
2015-08-14 15:13 ` john
0 siblings, 1 reply; 16+ messages in thread
From: list @ 2015-08-14 14:50 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/2] about: send images plain
2015-08-14 14:50 ` [PATCH 2/2] about: send images plain list
@ 2015-08-14 15:13 ` john
2015-08-14 21:16 ` [PATCH 2/3] " list
0 siblings, 1 reply; 16+ messages in thread
From: john @ 2015-08-14 15:13 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] about: send images plain
2015-08-14 15:13 ` john
@ 2015-08-14 21:16 ` list
2015-08-14 21:16 ` [PATCH 3/3] move shared code to get_mimetype_from_file() list
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: list @ 2015-08-14 21:16 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] move shared code to get_mimetype_from_file()
2015-08-14 21:16 ` [PATCH 2/3] " list
@ 2015-08-14 21:16 ` list
2015-08-15 11:29 ` john
2015-08-14 23:07 ` [PATCH 2/3] about: send images plain Jason
2015-08-15 11:30 ` john
2 siblings, 1 reply; 16+ messages in thread
From: list @ 2015-08-14 21:16 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] about: send images plain
2015-08-14 21:16 ` [PATCH 2/3] " list
2015-08-14 21:16 ` [PATCH 3/3] move shared code to get_mimetype_from_file() list
@ 2015-08-14 23:07 ` Jason
2015-08-15 20:10 ` list
2015-08-15 11:30 ` john
2 siblings, 1 reply; 16+ messages in thread
From: Jason @ 2015-08-14 23:07 UTC (permalink / raw)
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] move shared code to get_mimetype_from_file()
2015-08-14 21:16 ` [PATCH 3/3] move shared code to get_mimetype_from_file() list
@ 2015-08-15 11:29 ` john
2015-08-15 20:12 ` list
0 siblings, 1 reply; 16+ messages in thread
From: john @ 2015-08-15 11:29 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] about: send images plain
2015-08-14 21:16 ` [PATCH 2/3] " list
2015-08-14 21:16 ` [PATCH 3/3] move shared code to get_mimetype_from_file() list
2015-08-14 23:07 ` [PATCH 2/3] about: send images plain Jason
@ 2015-08-15 11:30 ` john
2015-08-15 20:08 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath() list
2 siblings, 1 reply; 16+ messages in thread
From: john @ 2015-08-15 11:30 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath()
2015-08-15 11:30 ` john
@ 2015-08-15 20:08 ` list
2015-08-15 20:08 ` [PATCH 3/3] ui-summary: send images plain for about page list
2015-08-16 12:19 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath() john
0 siblings, 2 replies; 16+ messages in thread
From: list @ 2015-08-15 20:08 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] ui-summary: send images plain for about page
2015-08-15 20:08 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath() list
@ 2015-08-15 20:08 ` list
2015-08-16 12:19 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath() john
1 sibling, 0 replies; 16+ messages in thread
From: list @ 2015-08-15 20:08 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] about: send images plain
2015-08-14 23:07 ` [PATCH 2/3] about: send images plain Jason
@ 2015-08-15 20:10 ` list
0 siblings, 0 replies; 16+ messages in thread
From: list @ 2015-08-15 20:10 UTC (permalink / raw)
"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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] move shared code to get_mimetype_from_file()
2015-08-15 11:29 ` john
@ 2015-08-15 20:12 ` list
0 siblings, 0 replies; 16+ messages in thread
From: list @ 2015-08-15 20:12 UTC (permalink / raw)
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath()
2015-08-15 20:08 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath() list
2015-08-15 20:08 ` [PATCH 3/3] ui-summary: send images plain for about page list
@ 2015-08-16 12:19 ` john
2015-08-16 12:51 ` list
1 sibling, 1 reply; 16+ messages in thread
From: john @ 2015-08-16 12:19 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath()
2015-08-16 12:19 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath() john
@ 2015-08-16 12:51 ` list
2015-08-16 12:53 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_for_filename() list
0 siblings, 1 reply; 16+ messages in thread
From: list @ 2015-08-16 12:51 UTC (permalink / raw)
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_for_filename()
2015-08-16 12:51 ` list
@ 2015-08-16 12:53 ` list
2015-08-16 12:53 ` [PATCH 3/3] ui-summary: send images plain for about page list
0 siblings, 1 reply; 16+ messages in thread
From: list @ 2015-08-16 12:53 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] ui-summary: send images plain for about page
2015-08-16 12:53 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_for_filename() list
@ 2015-08-16 12:53 ` list
0 siblings, 0 replies; 16+ messages in thread
From: list @ 2015-08-16 12:53 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-08-16 12:53 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-14 14:50 [PATCH 1/2] move get_mimetype_from_file() to shared list
2015-08-14 14:50 ` [PATCH 2/2] about: send images plain list
2015-08-14 15:13 ` john
2015-08-14 21:16 ` [PATCH 2/3] " list
2015-08-14 21:16 ` [PATCH 3/3] move shared code to get_mimetype_from_file() list
2015-08-15 11:29 ` john
2015-08-15 20:12 ` list
2015-08-14 23:07 ` [PATCH 2/3] about: send images plain Jason
2015-08-15 20:10 ` list
2015-08-15 11:30 ` john
2015-08-15 20:08 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath() list
2015-08-15 20:08 ` [PATCH 3/3] ui-summary: send images plain for about page list
2015-08-16 12:19 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_from_qrypath() john
2015-08-16 12:51 ` list
2015-08-16 12:53 ` [PATCH 2/3] refactor get_mimetype_from_file() to get_mimetype_for_filename() list
2015-08-16 12:53 ` [PATCH 3/3] ui-summary: send images plain for about page list
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).