[PATCH 1/3] Return const char * in cgit_{httpscheme, hosturl, rooturl}()

Mon Apr 1 21:51:38 CEST 2013

On Mon, Apr 01, 2013 at 04:35:35PM +0100, John Keeping wrote:
> On Mon, Apr 01, 2013 at 05:11:13PM +0200, Lukas Fleischer wrote:
> > The return values of these functions are essentially constant and should
> > never be modified.
> > 
> > Note that this will introduce a compiler warning when we try to free the
> > return value of any of these functions. However, given that all of these
> > currently return statically allocated strings in some cases, they need
> > to be refactored before this can be done anyway.
> 
> It looks like cgit_hosturl() already returns allocated memory in one
> case, and cgit_rooturl() returns a static buffer from fmt().
> 
> I wonder if it's sensible to just set ctx.env.http_host to something
> sensible in prepare_context() and get rid of cgit_hosturl completely,
> especially since it is used in all requests anyway.

Yeah, patch 1/3 was just (quick and dirty) preparatory work for patches
2/3 and 3/3. You should read the commit message like "This patch does it
wrong, but it helps with fixing the issues addressed in patches 2 and
3." -- I probably should have made it a bit clearer. A proper fix
requires a tiny bit more work, so feel free to come up with something to
base patches 2 and 3 on :)

> 
> I think we can also remove cgit_rooturl since in main() we set
> ctx.cfg.virtual_root to ctx.cfg.script_name in the same way this
> function does, so there's no need to fall back to script_name when we're
> rendering a page.
> 
> > Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> > ---
> >  ui-atom.c   | 4 ++--
> >  ui-shared.c | 8 ++++----
> >  ui-shared.h | 6 +++---
> >  3 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/ui-atom.c b/ui-atom.c
> > index 5b5525d..1554088 100644
> > --- a/ui-atom.c
> > +++ b/ui-atom.c
> > @@ -10,7 +10,7 @@
> >  #include "html.h"
> >  #include "ui-shared.h"
> >  
> > -static void add_entry(struct commit *commit, char *host)
> > +static void add_entry(struct commit *commit, const char *host)
> >  {
> >  	char delim = '&';
> >  	char *hex;
> > @@ -79,7 +79,7 @@ static void add_entry(struct commit *commit, char *host)
> >  
> >  void cgit_print_atom(char *tip, char *path, int max_count)
> >  {
> > -	char *host;
> > +	const char *host;
> >  	const char *argv[] = {NULL, tip, NULL, NULL, NULL};
> >  	struct commit *commit;
> >  	struct rev_info rev;
> > diff --git a/ui-shared.c b/ui-shared.c
> > index d4fb3d9..7a726c1 100644
> > --- a/ui-shared.c
> > +++ b/ui-shared.c
> > @@ -34,7 +34,7 @@ void cgit_print_error(const char *msg)
> >  	html("</div>\n");
> >  }
> >  
> > -char *cgit_httpscheme()
> > +const char *cgit_httpscheme()
> >  {
> >  	if (ctx.env.https && !strcmp(ctx.env.https, "on"))
> >  		return "https://";
> > @@ -42,7 +42,7 @@ char *cgit_httpscheme()
> >  		return "http://";
> >  }
> >  
> > -char *cgit_hosturl()
> > +const char *cgit_hosturl()
> >  {
> >  	if (ctx.env.http_host)
> >  		return ctx.env.http_host;
> > @@ -53,7 +53,7 @@ char *cgit_hosturl()
> >  	return xstrdup(fmt("%s:%s", ctx.env.server_name, ctx.env.server_port));
> >  }
> >  
> > -char *cgit_rooturl()
> > +const char *cgit_rooturl()
> >  {
> >  	if (ctx.cfg.virtual_root)
> >  		return fmt("%s/", ctx.cfg.virtual_root);
> > @@ -651,7 +651,7 @@ void cgit_print_docstart(struct cgit_context *ctx)
> >  		return;
> >  	}
> >  
> > -	char *host = cgit_hosturl();
> > +	const char *host = cgit_hosturl();
> >  	html(cgit_doctype);
> >  	html("<html xmlns='http://www.w3.org/1999/xhtml' xml:lang='en' lang='en'>\n");
> >  	html("<head>\n");
> > diff --git a/ui-shared.h b/ui-shared.h
> > index 87a7dac..0666388 100644
> > --- a/ui-shared.h
> > +++ b/ui-shared.h
> > @@ -1,9 +1,9 @@
> >  #ifndef UI_SHARED_H
> >  #define UI_SHARED_H
> >  
> > -extern char *cgit_httpscheme();
> > -extern char *cgit_hosturl();
> > -extern char *cgit_rooturl();
> > +extern const char *cgit_httpscheme();
> > +extern const char *cgit_hosturl();
> > +extern const char *cgit_rooturl();
> >  extern char *cgit_repourl(const char *reponame);
> >  extern char *cgit_fileurl(const char *reponame, const char *pagename,
> >  			  const char *filename, const char *query);
> > -- 
> > 1.8.2.411.g65a544e