Re: [PATCH 1/3] apply: get rid of useless x < 0 comparison on a size_t type
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:23
Ævar Arnfjörð Bjarmason [off-list ref] writes:
According to the C standard size_t is always unsigned, therefore the
comparison "n1 < 0 || n2 < 0" when n1 and n2 are size_t will always be
false.
This was raised by clang 2.9 which throws this warning when compiling
apply.c:
builtin/apply.c:253:9: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
if (n1 < 0 || n2 < 0)
~~ ^ ~
builtin/apply.c:253:19: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
if (n1 < 0 || n2 < 0)
~~ ^ ~
This check was originally added in v1.6.5-rc0~53^2 by Giuseppe Bilotta
while adding an option to git-apply to ignore whitespace differences.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---I agree that these quantities can never be negative, so I'll apply the patch as is. But I have this suspicion that this was a rather sloppy defensive check to protect this codepath against potential breakage in another codepath (most likely update_pre_post_images() touched by the same patch) that adjusts image->line[].len the caller of this function uses to feed these two parameters. Giuseppe may have been not confident enough that the code added to that function ensures not to undershoot when it reduces "len", or something. Giuseppe, can you explain what is going on? Thanks
quoted hunk
builtin/apply.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)diff --git a/builtin/apply.c b/builtin/apply.c index 84a8a0b..b3b59db 100644 --- a/builtin/apply.c +++ b/builtin/apply.c@@ -250,9 +250,6 @@ static int fuzzy_matchlines(const char *s1, size_t n1, const char *last2 = s2 + n2 - 1; int result = 0; - if (n1 < 0 || n2 < 0) - return 0; - /* ignore line endings */ while ((*last1 == '\r') || (*last1 == '\n')) last1--;