On Fri, 16 Sep 2011, Junio C Hamano wrote:
> Jakub Narebski <jnareb@xxxxxxxxx> writes:
> Micronit:
>
> > +# Sanitize for use in XHTML + application/xml+xhtm (valid XML 1.0)
> > +sub sanitize {
> > + my $str = shift;
> > +
> > + return undef unless defined $str;
>
> Given that the _whole_ point of this subroutine is to make $str safe for
> printing, wouldn't you want to either (1) die, declaring that feeding an
> undef to this subroutine is a programming error, or (2) return an empty
> string?
Well, that
return undef unless defined $str;
line is copy'n'paste (as is most of sanitize() body) from esc_html().
This line was added in 1df4876 (gitweb: Protect escaping functions against
calling on undef, 2010-02-07) with the following explanation
This is a bit of future-proofing esc_html and friends: when called
with undefined value they would now would return undef... which would
probably mean that error would still occur, but closer to the source
of problem.
This means that we can safely use
esc_html(shift) || "Internal Server Error"
in die_error() instead of
esc_html(shift || "Internal Server Error")
So actually now I see that while this line is good to have in esc_html(),
it is not really necessary in sanitize().
But anyway we don't want to replace undef with an empty string; undef is
(usually) an error, and we want to catch it, not to hide it.
> Given that the input to this function is from the result of feeding $line
> to untabify, which relies on $line being defined, and that $line comes
> from "while (my $line = <$fd>)" (and then chomp $line), it may be Ok for
> this subroutine to make the same assumption as untabify makes.
Right.
Passing undef to sanitize() is usually an error, and we don't want to hide
it. We want for gitweb test to detect it.
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html