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

2015-08-15 Thread Christian Hesse
John Keeping  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);}


pgpBQswV0vs_2.pgp
Description: OpenPGP digital signature
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 2/3] about: send images plain

2015-08-15 Thread Christian Hesse
"Jason A. Donenfeld"  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);}


pgpcRhjc7rebV.pgp
Description: OpenPGP digital signature
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


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

2015-08-15 Thread Christian Hesse
From: Christian Hesse 

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

Signed-off-by: Christian Hesse 
---
 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.c

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

2015-08-15 Thread Christian Hesse
From: Christian Hesse 

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 
---
 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

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 2/3] about: send images plain

2015-08-15 Thread John Keeping
On Fri, Aug 14, 2015 at 11:16:36PM +0200, Christian Hesse wrote:
> From: Christian Hesse 
> 
> 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 
> ---
>  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@lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


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

2015-08-15 Thread John Keeping
On Fri, Aug 14, 2015 at 11:16:37PM +0200, Christian Hesse wrote:
> From: Christian Hesse 
> 
> Signed-off-by: Christian Hesse 
> ---
>  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 = NU