Subject: Re: [PATCH 7/9] gitweb: cleanup error message produced by undefined $site_header

From: Jakub Narebski <[email-masked]>
Date: Fri, 15 Jan 2010 15:49:53 -0800 (PST)

"John 'Warthog9' Hawley" <[email-masked]> writes:

I have modified commit message a bit

> If $site_header is not defined you get extraneous errors in the web
> logs:
> 
> [Wed Jan 13 16:55:42 2010] [error] [client ::1] [Wed Jan 13 16:55:42 2010]
>  gitweb.cgi: Use of uninitialized value $site_header in -f at .../gitweb.cgi line 3287.,
>  referer: http://git/gitweb.cgi
> 
> for example (line wrapped for better readibility).  This commit
> ensures that the variable is defined before trying to use it.

  Ordinarily build procedure ensures that $site_header is defined (but
  empty, therefore false-ish), so this issue might happen only because
  of errors in gitweb config file.  Nevetheless it is better to code
  defensively.

  Signed-off-by: John 'Warthog9' Hawley <[email-masked]>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 07fdeb5..c4a177d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3277,7 +3277,7 @@ EOF
>  	print "</head>\n" .
>  	      "<body>\n";
>  
> -	if (-f $site_header) {
> +	if ($site_header && -f $site_header) {

It should be, I think

+	if (defined $site_header && -f $site_header) {

although I guess that nobody would name $site_header file "0".

>  		insert_file($site_header);
>  	}
>  
> -- 

More important is the fact that it is not the only variable holding
file name, that is not checked that it is defined before use.  You
should do the same change also for $site_footer and $home_text, and
die_error(500, "Gitweb misconfigured") or just die_error(500) if
$projects_list is not defined.

-- 
Jakub Narebski
Poland
ShadeHawk on #git